Horton

Re: pg_basebackup's --gzip switch misbehaves

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2022-09-13T07:13:20Z
Lists: pgsql-hackers

Attachments

On Sun, Sep 04, 2022 at 02:20:52PM +0900, Michael Paquier wrote:
> ffd5365 has missed that wal_compress_level should be set to
> Z_DEFAULT_COMPRESSION if there is nothing set in the compression
> spec for a zlib build.  pg_receivewal.c enforces that already.

So, I have looked at this one.  And it seems to me that the confusion
comes down to the existence of PG_COMPRESSION_OPTION_LEVEL.  I have
considered a couple of approaches here, like introducing an extra
routine in compression.c to assign a default compression level, but
my conclusion is at the end simpler: we always finish by setting up a
level even if the caller wants nothing, in which can we can just use
each library's default.  And lz4, zstd and zlib are able to handle the
case where a default is given down to their internal routines just
fine.

Attached is the patch I am finishing with, consisting of:
- the removal of PG_COMPRESSION_OPTION_LEVEL.
- assigning a default compression level when nothing is specified in
the spec.
- a couple of complifications in pg_receivewal, pg_basebackup and the
backend code as there is no need to worry about the compression
level.

A nice effect of this approach is that we can centralize the checks on
lz4, zstd and zlib when a build does not support any of these
options, as well as centralize the place where the default compression
levels are set.  This passes all the regression tests, and it fixes
the issue reported.  (Note that I have yet to run tests with all the
libraries disabled in ./configure.)

Thoughts?
--
Michael

Commits

  1. Make PG_TEST_NOCLEAN work for temporary directories in TAP tests

  2. doc: document the TAP test environment variables

  3. doc: update metacpan.org links to avoid redirects

  4. Use min/max bounds defined by Zstd for compression level

  5. Fix incorrect value for "strategy" with deflateParams() in walmethods.c

  6. Simplify handling of compression level with compression specifications