Horton

Re: refactoring basebackup.c

Robert Haas <robertmhaas@gmail.com>

View thread
From: Robert Haas <robertmhaas@gmail.com>
To: Dipesh Pandit <dipesh.pandit@gmail.com>, Andrew Dunstan <andrew@dunslane.net>
Cc: tushar <tushar.ahuja@enterprisedb.com>, Michael Paquier <michael@paquier.xyz>, Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>, Dmitry Dolgov <9erthalion6@gmail.com>, Mark Dilger <mark.dilger@enterprisedb.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2022-01-27T19:13: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.

Attachments

On Thu, Jan 27, 2022 at 2:37 AM Dipesh Pandit <dipesh.pandit@gmail.com> wrote:
> I have updated the patches to support server compression (gzip) for
> plain format backup. Please find attached v4 patches.

I made a pass over these patches today and made a bunch of minor
corrections. New version attached. The two biggest things I changed
are (1) s/gzip_extractor/gzip_compressor/, because I feel like you
extract an archive like a tarfile, but that is not what is happening
here, this is not an archive and (2) I took a few bits of out of the
test case that didn't seem to be necessary. There wasn't any reason
that I could see why testing for PG_VERSION needed to be skipped when
the compression method is 'none', so my first thought was to just take
out the 'if' statement around that, but then after more thought that
test and the one for pg_verifybackup are certainly going to fail if
those files are not present, so why have an extra test? It might make
sense if we were only conditionally able to run pg_verifybackup and
wanted to have some test coverage even when we can't, but that's not
the case here, so I see no point.

I studied this a bit to see whether I needed to make any adjustments
along the lines of 4f0bcc735038e96404fae59aa16ef9beaf6bb0aa in order
for this to work on msys. I think I don't, because 002_algorithm.pl
and 003_corruption.pl both pass $backup_path, not $real_backup_path,
to command_ok -- and I think something inside there does the
translation, which is weird, but we might as well be consistent.
008_untar.pl and 4f0bcc735038e96404fae59aa16ef9beaf6bb0aa needed to do
something different because --target server:X confused the msys magic,
but I think that shouldn't be an issue for this patch. However, I
might be wrong.

Barring objections or problems, I plan to commit this version
tomorrow. I'd do it today, but I have plans for tonight that are
incompatible with discovering that the build farm hates this ....

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