Re: Add LZ4 compression in pg_dump

gkokolatos@pm.me

From: gkokolatos@pm.me
To: Michael Paquier <michael@paquier.xyz>
Cc: Justin Pryzby <pryzby@telsasoft.com>, pgsql-hackers@lists.postgresql.org, Rachel Heaton <rachelmheaton@gmail.com>
Date: 2022-11-30T17:11:44Z
Lists: pgsql-hackers

Attachments





------- Original Message -------
On Wednesday, November 30th, 2022 at 1:50 AM, Michael Paquier <michael@paquier.xyz> wrote:


> 
> 
> On Tue, Nov 29, 2022 at 12:10:46PM +0000, gkokolatos@pm.me wrote:
> 
> > Thank you. Please advice if is preferable to split 0002 in two parts.
> > I think not but I will happily do so if you think otherwise.
> 
> 
> This one makes me curious. What kind of split are you talking about?
> If it makes the code review and the git history cleaner and easier, I
> am usually a lot in favor of such incremental changes. As far as I
> can see, there is the switch from the compression integer to
> compression specification as one thing. The second thing is the
> refactoring of cfclose() and these routines, paving the way for 0003.
> Hmm, it may be cleaner to move the switch to the compression spec in
> one patch, and move the logic around cfclose() to its own, paving the
> way to 0003.

Fair enough. The atteched v11 does that. 0001 introduces compression
specification and is using it throughout. 0002 paves the way to the
new interface by homogenizing the use of cfp. 0003 introduces the new
API and stores the compression algorithm in the custom format header
instead of the compression level integer. Finally 0004 adds support for
LZ4.

Besides the version bump in 0003 which can possibly be split out and
as an independent and earlier step, I think that the patchset consists
of coherent units.

> By the way, I think that this 0002 should drop all the default clauses
> in the switches for the compression method so as we'd catch any
> missing code paths with compiler warnings if a new compression method
> is added in the future.

Sure. 

> Anyway, I have applied 0001, adding you as a primary author because
> you did most of it with only tweaks from me for pg_basebackup. The
> docs of pg_basebackup have been amended to mention the slight change
> in grammar, affecting the case where we do not have a detail string.

Very kind of you, thank you.

Cheers,
//Georgios

> --
> Michael

Commits

  1. Advance input pointer when LZ4 compressing data

  2. Null-terminate the output buffer of LZ4Stream_gets

  3. Rework code defining default compression for dir/custom formats in pg_dump

  4. pg_dump: Use only LZ4 frame format for compression

  5. Minor comment improvements for compress_lz4

  6. Unify buffer sizes in pg_dump compression API

  7. Improve type handling in pg_dump's compress file API

  8. Improve wording in pg_dump compression docs

  9. Fix condition in pg_dump TAP test

  10. Add LZ4 compression to pg_dump

  11. Introduce a generic pg_dump compression API

  12. Prepare pg_dump internals for additional compression methods

  13. Fix behavior with pg_restore -l and compressed dumps

  14. Add 250c8ee07ed to git-blame-ignore-revs

  15. Provide test coverage in pg_dump for default behaviors with compression

  16. Switch pg_dump to use compression specifications

  17. Refactor code parsing compression option values (-Z/--compress)

  18. meson: Add some missing env settings for tests of pg_dump and pg_verifybackup

  19. Extend TAP tests of pg_dump to test for compression with gzip

  20. Clean up some dead code in pg_dump with tar format and gzip compression

  21. Add TAP test in pg_dump with --format=tar and --compress

  22. Replace BASE_BACKUP COMPRESSION_LEVEL option with COMPRESSION_DETAIL.

  23. Refactor the pg_dump zlib code from pg_backup_custom.c to a separate file,