Horton

Re: Add LZ4 compression in pg_dump

Justin Pryzby <pryzby@telsasoft.com>

View thread View on postgr.es
From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: gkokolatos@pm.me, Tomas Vondra <tomas.vondra@enterprisedb.com>, pgsql-hackers@lists.postgresql.org, Rachel Heaton <rachelmheaton@gmail.com>
Date: 2023-01-26T06:28:46Z
Lists: pgsql-hackers 
On Thu, Jan 26, 2023 at 02:49:27PM +0900, Michael Paquier wrote:
> On Wed, Jan 25, 2023 at 12:00:20PM -0600, Justin Pryzby wrote:
> > While looking at this, I realized that commit 5e73a6048 introduced a
> > regression:
> > 
> > @@ -3740,19 +3762,24 @@ ReadHead(ArchiveHandle *AH)
> > 
> > -       if (AH->compression != 0)
> > -               pg_log_warning("archive is compressed, but this installation does not support compression -- no data will be available");
> > +       if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
> > +               pg_fatal("archive is compressed, but this installation does not support compression");
> > 
> > Before, it was possible to restore non-data chunks of a dump file, even
> > if the current build didn't support its compression.  But that's now
> > impossible - and it makes the code we're discussing in RestoreArchive()
> > unreachable.
> 
> Right.  The impacts the possibility of looking at the header data,
> which is useful with pg_restore -l for example.

It's not just header data - it's schema and (I think) everything other
than table data.

> > The coverage report disagrees with me, though...
> > https://coverage.postgresql.org/src/bin/pg_dump/pg_backup_archiver.c.gcov.html#3901
> 
> Isn't that one of the tests like compression_gzip_plain?

I'm not sure what you mean.  Plain dump is restored with psql and not
with pg_restore.

My line number was wrong:
https://coverage.postgresql.org/src/bin/pg_dump/pg_backup_archiver.c.gcov.html#390

What test would hit that code without rebuilding ?

394             : #ifndef HAVE_LIBZ
395             :     if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP &&

> Thoughts?
>  #ifndef HAVE_LIBZ
>  	if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
> -		pg_fatal("archive is compressed, but this installation does not support compression");
> +		pg_log_warning("archive is compressed, but this installation does not support compression -- no data will be available");

Your patch is fine for now, but these errors should eventually specify
*which* compression algorithm is unavailable.  I think that should be a
part of the 001 patch, ideally in a way that minimizes the number of
places which need to be updated when adding an algorithm.

-- 
Justin

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,