Horton

Re: refactoring basebackup.c

Robert Haas <robertmhaas@gmail.com>

View thread
From: Robert Haas <robertmhaas@gmail.com>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, tushar <tushar.ahuja@enterprisedb.com>
Date: 2021-07-21T15:09:08Z
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, Jul 20, 2021 at 4:03 PM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
> I was only imagining having a callback for injecting manifests or recovery configurations.  It is not necessary that this be done in the current patch set, or perhaps ever.

A callback where?

I actually think the ideal scenario would be if the server always did
all the work and the client wasn't involved in editing the tarfile,
but it's not super-easy to get there from here. We could add an option
to tell the server whether to inject the manifest into the archive,
which probably wouldn't be too bad. For it to inject the recovery
configuration, we'd have to send that configuration to the server
somehow. I thought about using COPY BOTH mode instead of COPY OUT mode
to allow for stuff like that, but it seems pretty complicated, and I
wasn't really sure that we'd get consensus that it was better even if
I went to the trouble of coding it up.

If we don't do that and stick with the current system where it's
handled on the client side, then I agree that we want to separate the
tar-specific concerns from the injection-type concerns, which the
patch does by making those operations different kinds of bbstreamer
that know only a relatively limited amount about what each other are
doing. You get [server] => [tar parser] => [recovery injector] => [tar
archiver], where the [recovery injector] step nukes the archive file
headers for the files it adds or modifies, and the [tar archiver] step
fixes them up again. So the only thing that the [recovery injector]
piece needs to know is that if it makes any changes to a file, it
should send that file to the next step with a 0-length archive header,
and all the [tar archiver] piece needs to know is that already-valid
headers can be left alone and 0-length ones need to be regenerated.

There may be a better scheme; I don't think this is perfectly elegant.
I do think it's better than what we've got now.

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