pgsql: Simplify handling of compression level with compression specific - Mailing list pgsql-committers

From Michael Paquier
Subject pgsql: Simplify handling of compression level with compression specific
Date
Msg-id E1oYIua-000FJc-Tg@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Simplify handling of compression level with compression specifications

PG_COMPRESSION_OPTION_LEVEL is removed from the compression
specification logic, and instead the compression level is always
assigned with each library's default if nothing is directly given.  This
centralizes the checks on the compression methods supported by a given
build, and always assigns a default compression level when parsing a
compression specification.  This results in complaining at an earlier
stage than previously if a build supports a compression method or not,
aka when parsing a specification in the backend or the frontend, and not
when processing it.  zstd, lz4 and zlib are able to handle in their
respective routines setting up the compression level the case of a
default value, hence the backend or frontend code (pg_receivewal or
pg_basebackup) has now no need to know what the default compression
level should be if nothing is specified: the logic is now done so as the
specification parsing assigns it.  It can also be enforced by passing
down a "level" set to the default value, that the backend will accept
(the replication protocol is for example able to handle a command like
BASE_BACKUP (COMPRESSION_DETAIL 'gzip:level=-1')).

This code simplification fixes an issue with pg_basebackup --gzip
introduced by ffd5365, where the tarball of the streamed WAL segments
would be created as of pg_wal.tar.gz with uncompressed contents, while
the intention is to compress the segments with gzip at a default level.
The origin of the confusion comes from the handling of the default
compression level of gzip (-1 or Z_DEFAULT_COMPRESSION) and the value of
0 was getting assigned, which is what walmethods.c would consider
as equivalent to no compression when streaming WAL segments with its tar
methods.  Assigning always the compression level removes the confusion
of some code paths considering a value of 0 set in a specification as
either no compression or a default compression level.

Note that 010_pg_basebackup.pl has to be adjusted to skip a few tests
where the shape of the compression detail string for client and
server-side compression was checked using gzip.  This is a result of the
code simplification, as gzip specifications cannot be used if a build
does not support it.

Reported-by: Tom Lane
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/1400032.1662217889@sss.pgh.pa.us
Backpatch-through: 15

Branch
------
REL_15_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/53332eacaff6a391f79294193d0c39bff9d14b43

Modified Files
--------------
doc/src/sgml/protocol.sgml                   |  10 +-
src/backend/backup/basebackup_gzip.c         |  10 +-
src/backend/backup/basebackup_lz4.c          |   9 +-
src/backend/backup/basebackup_zstd.c         |  13 +--
src/bin/pg_basebackup/bbstreamer_gzip.c      |   4 +-
src/bin/pg_basebackup/bbstreamer_lz4.c       |   3 +-
src/bin/pg_basebackup/bbstreamer_zstd.c      |  15 ++-
src/bin/pg_basebackup/pg_basebackup.c        |   7 +-
src/bin/pg_basebackup/pg_receivewal.c        |  35 +------
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 134 ++++++++++++++-------------
src/common/compression.c                     | 105 ++++++++++++++++-----
src/include/common/compression.h             |   3 +-
12 files changed, 189 insertions(+), 159 deletions(-)


pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: Re: pgsql: aix: No need to use mkldexport when we want to export all symbol
Next
From: John Naylor
Date:
Subject: pgsql: Move gramparse.h to src/backend/parser