Horton

Re: Add LZ4 compression in pg_dump

gkokolatos@pm.me

View thread View on postgr.es
From: gkokolatos@pm.me
To: Alexander Lakhin <exclusion@gmail.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>, Justin Pryzby <pryzby@telsasoft.com>, shiy.fnst@fujitsu.com, Michael Paquier <michael@paquier.xyz>, pgsql-hackers@lists.postgresql.org, Rachel Heaton <rachelmheaton@gmail.com>
Date: 2023-05-05T10:02:12Z
Lists: pgsql-hackers

Attachments





------- Original Message -------
On Friday, May 5th, 2023 at 8:00 AM, Alexander Lakhin <exclusion@gmail.com> wrote:


> 
> 
> 23.03.2023 20:10, Tomas Vondra wrote:
> 
> > So pushed all three parts, after updating the commit messages a bit.
> > 
> > This leaves the empty-data issue (which we have a fix for) and the
> > switch to LZ4F. And then the zstd part.
> 
> 
> I'm sorry that I haven't noticed/checked that before, but when trying to
> perform check-world with Valgrind I've discovered another issue presumably
> related to LZ4File_gets().
> When running under Valgrind:
> PROVE_TESTS=t/002_pg_dump.pl make check -C src/bin/pg_dump/
> I get:
> ...
> 07:07:11.683 ok 1939 - compression_lz4_dir: glob check for
> .../src/bin/pg_dump/tmp_check/tmp_test_HB6A/compression_lz4_dir/*.dat.lz4
> # Running: pg_restore --jobs=2 --file=.../src/bin/pg_dump/tmp_check/tmp_test_HB6A/compression_lz4_dir.sql
> .../src/bin/pg_dump/tmp_check/tmp_test_HB6A/compression_lz4_dir
> 
> ==00:00:00:00.579 2811926== Conditional jump or move depends on uninitialised value(s)
> ==00:00:00:00.579 2811926== at 0x4853376: rawmemchr (vg_replace_strmem.c:1548)
> ==00:00:00:00.579 2811926== by 0x4C96A67: _IO_str_init_static_internal (strops.c:41)
> ==00:00:00:00.579 2811926== by 0x4C693A2: _IO_strfile_read (strfile.h:95)
> ==00:00:00:00.579 2811926== by 0x4C693A2: __isoc99_sscanf (isoc99_sscanf.c:28)
> ==00:00:00:00.579 2811926== by 0x11DB6F: _LoadLOs (pg_backup_directory.c:458)
> ==00:00:00:00.579 2811926== by 0x11DD1E: _PrintTocData (pg_backup_directory.c:422)
> ==00:00:00:00.579 2811926== by 0x118484: restore_toc_entry (pg_backup_archiver.c:882)
> ==00:00:00:00.579 2811926== by 0x1190CC: RestoreArchive (pg_backup_archiver.c:699)
> ==00:00:00:00.579 2811926== by 0x10F25D: main (pg_restore.c:414)
> ==00:00:00:00.579 2811926==
> ...
> 
> It looks like the line variable returned by gets_func() here is not
> null-terminated:
> while ((CFH->gets_func(line, MAXPGPATH, CFH)) != NULL)
> 
> {
> ...
> if (sscanf(line, "%u %" CppAsString2(MAXPGPATH) "s\n", &oid, lofname) != 2)
> ...
> And Valgrind doesn't like it.
> 

Valgrind is correct to not like it. LZ4Stream_gets() got modeled after
gets() when it should have been modeled after fgets().

Please find a patch attached to address it.

Cheers,
//Georgios

> Best regards,
> Alexander

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,