Horton

Re: refactoring basebackup.c

Robert Haas <robertmhaas@gmail.com>

View thread
From: Robert Haas <robertmhaas@gmail.com>
To: Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>
Cc: Dipesh Pandit <dipesh.pandit@gmail.com>, Andrew Dunstan <andrew@dunslane.net>, tushar <tushar.ahuja@enterprisedb.com>, Michael Paquier <michael@paquier.xyz>, Dmitry Dolgov <9erthalion6@gmail.com>, Mark Dilger <mark.dilger@enterprisedb.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2022-01-28T19:49:48Z
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 Fri, Jan 28, 2022 at 12:48 PM Jeevan Ladhe
<jeevan.ladhe@enterprisedb.com> wrote:
> I have attached the latest rebased version of the LZ4 server-side compression
> patch on the recent commits. This patch also introduces the compression level
> and adds a tap test.

In view of this morning's commit of
d45099425eb19e420433c9d81d354fe585f4dbd6 I think the threshold for
committing this patch has gone up. We need to make it support
decompression with LZ4 on the client side, as we now have for gzip.

Other comments:

- Even if we were going to support LZ4 only on the server side, surely
it's not right to refuse --compress lz4 and --compress client-lz4 at
the parsing stage. I don't even think the message you added to main()
is reachable.

- In the new test case you set decompress_flags but according to the
documentation I have here, -m is for multiple files (and so should not
be needed here) and -d is for decompression (which is what we want
here). So I'm confused why this is like this.

Other than that this seems like it's in pretty good shape.

> Also, while adding the lz4 case in the pg_verifybackup/t/008_untar.pl, I found
> an unused variable {have_zlib}. I have attached a cleanup patch for that as well.

This part seems clearly correct, so I have committed it.

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