Horton

Re: Add LZ4 compression in pg_dump

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: gkokolatos@pm.me
Cc: Justin Pryzby <pryzby@telsasoft.com>, Rachel Heaton <rachelmheaton@gmail.com>, Greg Stark <stark@mit.edu>, pgsql-hackers@lists.postgresql.org
Date: 2022-04-05T01:34:49Z
Lists: pgsql-hackers

Attachments

On Fri, Apr 01, 2022 at 03:06:40PM +0000, gkokolatos@pm.me wrote:
> I understand the itch. Indeed when LZ4 is added as compression method, this
> block changes slightly. I went with the minimum amount changed. Please find
> in 0001 of the attached this variable renamed as $gzip_program_exist. I thought
> that as prefix it will match better the already used $ENV{GZIP_PROGRAM}.

Hmm.  I have spent some time on that, and upon review I really think
that we should skip the tests marked as dedicated to the gzip
compression entirely if the build is not compiled with this option,
rather than letting the code run a dump for nothing in some cases,
relying on the default to uncompress the contents in others.  In the
latter case, it happens that we have already some checks like
defaults_custom_format, but you already mentioned that.

We should also skip the later parts of the tests if the compression
program does not exist as we rely on it, but only if the command does
not exist.  This will count for LZ4.

> I can see the overlap case. Yet, I understand the test_key as serving different
> purpose, as it is a key of %tests and %full_runs. I do not expect the database
> content of the generated dump to change based on which compression method is used.

Contrary to the current LZ4 tests in pg_dump, what we have here is a
check for a command-level run and not a data-level check.  So what's
introduced is a new concept, and we need a new way to control if the
tests should be entirely skipped or not, particularly if we finish by
not using test_key to make the difference.  Perhaps the best way to
address that is to have a new keyword in the $runs structure.  The
attached defines a new compile_option, that can be completed later for
new compression methods introduced in the tests.  So the idea is to
mark all the tests related to compression with the same test_key, and
the tests can be skipped depending on what compile_option requires.

> In the attached version, I propose that the compression_cmd is converted into
> a hash. It contains two keys, the program and the arguments. Maybe it is easier
> to read than before or than simply grabbing the first element of the array.

Splitting the program and its arguments makes sense.

At the end I am finishing with the attached.  I also saw an overlap
with the addition of --jobs for the directory format vs not using the
option, so I have removed the case where --jobs was not used in the
directory format.
--
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,