Re: pg_basebackup's --gzip switch misbehaves

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: pg_basebackup's --gzip switch misbehaves
Date: 2022-09-13 07:13:20
Message-ID: YyAtkOU/C/SD9qlL@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-Simplify-compression-level-handling-in-compression-s.patch text/x-diff 14.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2022-09-13 07:15:34 Re: Error "initial slot snapshot too large" in create replication slot
Previous Message Kyotaro Horiguchi 2022-09-13 07:10:59 Re: Error "initial slot snapshot too large" in create replication slot