Re: refactoring basebackup.c

Robert Haas <robertmhaas@gmail.com>

From: Robert Haas <robertmhaas@gmail.com>
To: Jeevan Ladhe <jeevanladhe.os@gmail.com>
Cc: Dipesh Pandit <dipesh.pandit@gmail.com>, Andres Freund <andres@anarazel.de>, Justin Pryzby <pryzby@telsasoft.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, "Shinoda, Noriyoshi (PN Japan FSIP)" <noriyoshi.shinoda@hpe.com>, Abhijit Menon-Sen <ams@toroid.org>, Dmitry Dolgov <9erthalion6@gmail.com>, Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>, Mark Dilger <mark.dilger@enterprisedb.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, tushar <tushar.ahuja@enterprisedb.com>
Date: 2022-03-15T17:50:36Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Document BaseBackupSync and BaseBackupWrite wait events.

  2. Support long distance matching for zstd compression

  3. Fix possible NULL-pointer-deference in backup_compression.c.

  4. Allow parallel zstd compression when taking a base backup.

  5. Make PostgreSQL::Test::Cluster::run_log() return a useful value.

  6. Fix a few goofs in new backup compression code.

  7. Replace BASE_BACKUP COMPRESSION_LEVEL option with COMPRESSION_DETAIL.

  8. Add 'basebackup_to_shell' contrib module.

  9. Allow extensions to add new backup targets.

  10. Change HAVE_LIBLZ4 and HAVE_LIBZSTD tests to USE_LZ4 and USE_ZSTD.

  11. pg_basebackup: Clean up some bogus file extension tests.

  12. pg_basebackup: Avoid unclean failure with server-compression and -D -.

  13. Fix LZ4 tests for remaining buffer space.

  14. Add support for zstd base backup compression.

  15. pg_basebackup: Allow client-side LZ4 (de)compression.

  16. Add suport for server-side LZ4 base backup compression.

  17. Add min() and max() aggregates for xid8.

  18. Remove superfluous variable.

  19. pg_basebackup: Cleaner handling when compression is multiply specified.

  20. Allow server-side compression to be used with -Fp.

  21. pg_basebackup: Fix a couple of recently-introduced bugs.

  22. Tidy up a few cosmetic issues related to pg_basebackup.

  23. Server-side gzip compression.

  24. Unbreak pg_basebackup/t/010_pg_basebackup.pl on msys

  25. Suppress variable-set-but-not-used warning from clang 13.

  26. Extend the options of pg_basebackup to control compression

  27. Support base backup targets.

  28. Modify pg_basebackup to use a new COPY subprotocol for base backups.

  29. Document that tar archives are now properly terminated.

  30. Fix thinko in bbsink_throttle_manifest_contents.

  31. Have the server properly terminate tar archives.

  32. Minimal fix for unterminated tar archive problem.

  33. Introduce 'bbstreamer' abstraction to modularize pg_basebackup.

  34. Introduce 'bbsink' abstraction to modularize base backup code.

  35. Refactor basebackup.c's _tarWriteDir() function.

  36. Flexible options for CREATE_REPLICATION_SLOT.

  37. Flexible options for BASE_BACKUP.

On Tue, Mar 15, 2022 at 6:33 AM Jeevan Ladhe <jeevanladhe.os@gmail.com> wrote:
> I get following error at my end:
>
> $ pg_basebackup -D /tmp/zstd_bk -Ft -Xfetch --compress=server-zstd:7@4
> pg_basebackup: error: could not initiate base backup: ERROR:  could not compress data: Unsupported parameter
> pg_basebackup: removing data directory "/tmp/zstd_bk"
>
> This is mostly because I have the zstd library version v1.4.4, which
> does not have default support for parallel workers. Maybe we should
> have a better error, something that is hinting that the parallelism is
> not supported by the particular build.

I'm not averse to trying to improve that error message, but honestly
I'd consider that to be good enough already to be acceptable. We could
think about trying to add an errhint() telling you that the problem
may be with your libzstd build.

> The regression for pg_verifybackup test 008_untar.pl also fails with a
> similar error. Here, I think we should have some logic in regression to
> skip the test if the parameter is not supported?

Or at least to have the test not fail.

> Also, just a thought, for the versions where parallelism is not
> supported, should we instead just throw a warning and fall back to
> non-parallel behavior?

I don't think so. I think it's better for the user to get an error and
then change their mind and request something we can do.

-- 
Robert Haas
EDB: http://www.enterprisedb.com