Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf352e2d

Browse files
committed
Simplify handling of compression level with compression specifications
PG_COMPRESSION_OPTION_LEVEL is removed from the compressionspecification logic, and instead the compression level is alwaysassigned with each library's default if nothing is directly given. Thiscentralizes the checks on the compression methods supported by a givenbuild, and always assigns a default compression level when parsing acompression specification. This results in complaining at an earlierstage than previously if a build supports a compression method or not,aka when parsing a specification in the backend or the frontend, and notwhen processing it. zstd, lz4 and zlib are able to handle in theirrespective routines setting up the compression level the case of adefault value, hence the backend or frontend code (pg_receivewal orpg_basebackup) has now no need to know what the default compressionlevel should be if nothing is specified: the logic is now done so as thespecification parsing assigns it. It can also be enforced by passingdown a "level" set to the default value, that the backend will accept(the replication protocol is for example able to handle a command likeBASE_BACKUP (COMPRESSION_DETAIL 'gzip:level=-1')).This code simplification fixes an issue with pg_basebackup --gzipintroduced byffd5365, where the tarball of the streamed WAL segmentswould be created as of pg_wal.tar.gz with uncompressed contents, whilethe intention is to compress the segments with gzip at a default level.The origin of the confusion comes from the handling of the defaultcompression level of gzip (-1 or Z_DEFAULT_COMPRESSION) and the value of0 was getting assigned, which is what walmethods.c would consideras equivalent to no compression when streaming WAL segments with its tarmethods. Assigning always the compression level removes the confusionof some code paths considering a value of 0 set in a specification aseither no compression or a default compression level.Note that 010_pg_basebackup.pl has to be adjusted to skip a few testswhere the shape of the compression detail string for client andserver-side compression was checked using gzip. This is a result of thecode simplification, as gzip specifications cannot be used if a builddoes not support it.Reported-by: Tom LaneReviewed-by: Tom LaneDiscussion:https://postgr.es/m/1400032.1662217889@sss.pgh.pa.usBackpatch-through: 15
1 parent0a20ff5 commitf352e2d

File tree

12 files changed

+189
-159
lines changed

12 files changed

+189
-159
lines changed

‎doc/src/sgml/protocol.sgml‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2752,9 +2752,13 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
27522752
<para>
27532753
The <literal>level</literal> keyword sets the compression level.
27542754
For <literal>gzip</literal> the compression level should be an
2755-
integer between 1 and 9, for <literal>lz4</literal> an integer
2756-
between 1 and 12, and for <literal>zstd</literal> an integer
2757-
between 1 and 22.
2755+
integer between <literal>1</literal> and <literal>9</literal>
2756+
(default <literal>Z_DEFAULT_COMPRESSION</literal> or
2757+
<literal>-1</literal>), for <literal>lz4</literal> an integer
2758+
between 1 and 12 (default <literal>0</literal> for fast compression
2759+
mode), and for <literal>zstd</literal> an integer between
2760+
<literal>1</literal> and <literal>22</literal> (default
2761+
<literal>ZSTD_CLEVEL_DEFAULT</literal> or <literal>3</literal>).
27582762
</para>
27592763

27602764
<para>

‎src/backend/backup/basebackup_gzip.c‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,9 @@ bbsink_gzip_new(bbsink *next, pg_compress_specification *compress)
7272

7373
Assert(next!=NULL);
7474

75-
if ((compress->options&PG_COMPRESSION_OPTION_LEVEL)==0)
76-
compresslevel=Z_DEFAULT_COMPRESSION;
77-
else
78-
{
79-
compresslevel=compress->level;
80-
Assert(compresslevel >=1&&compresslevel <=9);
81-
}
75+
compresslevel=compress->level;
76+
Assert((compresslevel >=1&&compresslevel <=9)||
77+
compresslevel==Z_DEFAULT_COMPRESSION);
8278

8379
sink=palloc0(sizeof(bbsink_gzip));
8480
*((constbbsink_ops**)&sink->base.bbs_ops)=&bbsink_gzip_ops;

‎src/backend/backup/basebackup_lz4.c‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,8 @@ bbsink_lz4_new(bbsink *next, pg_compress_specification *compress)
7272

7373
Assert(next!=NULL);
7474

75-
if ((compress->options&PG_COMPRESSION_OPTION_LEVEL)==0)
76-
compresslevel=0;
77-
else
78-
{
79-
compresslevel=compress->level;
80-
Assert(compresslevel >=1&&compresslevel <=12);
81-
}
75+
compresslevel=compress->level;
76+
Assert(compresslevel >=0&&compresslevel <=12);
8277

8378
sink=palloc0(sizeof(bbsink_lz4));
8479
*((constbbsink_ops**)&sink->base.bbs_ops)=&bbsink_lz4_ops;

‎src/backend/backup/basebackup_zstd.c‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,14 +96,11 @@ bbsink_zstd_begin_backup(bbsink *sink)
9696
if (!mysink->cctx)
9797
elog(ERROR,"could not create zstd compression context");
9898

99-
if ((compress->options&PG_COMPRESSION_OPTION_LEVEL)!=0)
100-
{
101-
ret=ZSTD_CCtx_setParameter(mysink->cctx,ZSTD_c_compressionLevel,
102-
compress->level);
103-
if (ZSTD_isError(ret))
104-
elog(ERROR,"could not set zstd compression level to %d: %s",
105-
compress->level,ZSTD_getErrorName(ret));
106-
}
99+
ret=ZSTD_CCtx_setParameter(mysink->cctx,ZSTD_c_compressionLevel,
100+
compress->level);
101+
if (ZSTD_isError(ret))
102+
elog(ERROR,"could not set zstd compression level to %d: %s",
103+
compress->level,ZSTD_getErrorName(ret));
107104

108105
if ((compress->options&PG_COMPRESSION_OPTION_WORKERS)!=0)
109106
{

‎src/bin/pg_basebackup/bbstreamer_gzip.c‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,7 @@ bbstreamer_gzip_writer_new(char *pathname, FILE *file,
107107
pg_fatal("could not open output file: %m");
108108
}
109109

110-
if ((compress->options&PG_COMPRESSION_OPTION_LEVEL)!=0&&
111-
gzsetparams(streamer->gzfile,compress->level,
112-
Z_DEFAULT_STRATEGY)!=Z_OK)
110+
if (gzsetparams(streamer->gzfile,compress->level,Z_DEFAULT_STRATEGY)!=Z_OK)
113111
pg_fatal("could not set compression level %d: %s",
114112
compress->level,get_gz_error(streamer->gzfile));
115113

‎src/bin/pg_basebackup/bbstreamer_lz4.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,7 @@ bbstreamer_lz4_compressor_new(bbstreamer *next, pg_compress_specification *compr
8888
prefs=&streamer->prefs;
8989
memset(prefs,0,sizeof(LZ4F_preferences_t));
9090
prefs->frameInfo.blockSizeID=LZ4F_max256KB;
91-
if ((compress->options&PG_COMPRESSION_OPTION_LEVEL)!=0)
92-
prefs->compressionLevel=compress->level;
91+
prefs->compressionLevel=compress->level;
9392

9493
ctxError=LZ4F_createCompressionContext(&streamer->cctx,LZ4F_VERSION);
9594
if (LZ4F_isError(ctxError))

‎src/bin/pg_basebackup/bbstreamer_zstd.c‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,12 @@ bbstreamer_zstd_compressor_new(bbstreamer *next, pg_compress_specification *comp
8484
if (!streamer->cctx)
8585
pg_fatal("could not create zstd compression context");
8686

87-
/* Set compression level, if specified */
88-
if ((compress->options&PG_COMPRESSION_OPTION_LEVEL)!=0)
89-
{
90-
ret=ZSTD_CCtx_setParameter(streamer->cctx,ZSTD_c_compressionLevel,
91-
compress->level);
92-
if (ZSTD_isError(ret))
93-
pg_fatal("could not set zstd compression level to %d: %s",
94-
compress->level,ZSTD_getErrorName(ret));
95-
}
87+
/* Set compression level */
88+
ret=ZSTD_CCtx_setParameter(streamer->cctx,ZSTD_c_compressionLevel,
89+
compress->level);
90+
if (ZSTD_isError(ret))
91+
pg_fatal("could not set zstd compression level to %d: %s",
92+
compress->level,ZSTD_getErrorName(ret));
9693

9794
/* Set # of workers, if specified */
9895
if ((compress->options&PG_COMPRESSION_OPTION_WORKERS)!=0)

‎src/bin/pg_basebackup/pg_basebackup.c‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2012,9 +2012,7 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
20122012
if (client_compress->algorithm==PG_COMPRESSION_GZIP)
20132013
{
20142014
wal_compress_algorithm=PG_COMPRESSION_GZIP;
2015-
wal_compress_level=
2016-
(client_compress->options&PG_COMPRESSION_OPTION_LEVEL)
2017-
!=0 ?client_compress->level :0;
2015+
wal_compress_level=client_compress->level;
20182016
}
20192017
else
20202018
{
@@ -2023,7 +2021,8 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
20232021
}
20242022

20252023
StartLogStreamer(xlogstart,starttli,sysidentifier,
2026-
wal_compress_algorithm,wal_compress_level);
2024+
wal_compress_algorithm,
2025+
wal_compress_level);
20272026
}
20282027

20292028
if (serverMajor >=1500)

‎src/bin/pg_basebackup/pg_receivewal.c‎

Lines changed: 4 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -875,38 +875,11 @@ main(int argc, char **argv)
875875
pg_fatal("invalid compression specification: %s",
876876
error_detail);
877877

878-
/* Extract the compression level, if found in the specification */
879-
if ((compression_spec.options&PG_COMPRESSION_OPTION_LEVEL)!=0)
880-
compresslevel=compression_spec.level;
881-
882-
switch (compression_algorithm)
883-
{
884-
casePG_COMPRESSION_NONE:
885-
/* nothing to do */
886-
break;
887-
casePG_COMPRESSION_GZIP:
888-
#ifdefHAVE_LIBZ
889-
if ((compression_spec.options&PG_COMPRESSION_OPTION_LEVEL)==0)
890-
{
891-
pg_log_info("no value specified for --compress, switching to default");
892-
compresslevel=Z_DEFAULT_COMPRESSION;
893-
}
894-
#else
895-
pg_fatal("this build does not support compression with %s",
896-
"gzip");
897-
#endif
898-
break;
899-
casePG_COMPRESSION_LZ4:
900-
#ifndefUSE_LZ4
901-
pg_fatal("this build does not support compression with %s",
902-
"LZ4");
903-
#endif
904-
break;
905-
casePG_COMPRESSION_ZSTD:
906-
pg_fatal("compression with %s is not yet supported","ZSTD");
907-
break;
908-
}
878+
/* Extract the compression level */
879+
compresslevel=compression_spec.level;
909880

881+
if (compression_algorithm==PG_COMPRESSION_ZSTD)
882+
pg_fatal("compression with %s is not yet supported","ZSTD");
910883

911884
/*
912885
* Check existence of destination folder.

‎src/bin/pg_basebackup/t/010_pg_basebackup.pl‎

Lines changed: 72 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -86,71 +86,81 @@
8686
# Now that we have a server that supports replication commands, test whether
8787
# certain invalid compression commands fail on the client side with client-side
8888
# compression and on the server side with server-side compression.
89-
my$client_fails ='pg_basebackup: error:';
90-
my$server_fails =
91-
'pg_basebackup: error: could not initiate base backup: ERROR:';
92-
my@compression_failure_tests = (
93-
[
94-
'extrasquishy',
95-
'unrecognized compression algorithm "extrasquishy"',
96-
'failure on invalid compression algorithm'
97-
],
98-
[
99-
'gzip:',
100-
'invalid compression specification: found empty string where a compression option was expected',
101-
'failure on empty compression options list'
102-
],
103-
[
104-
'gzip:thunk',
105-
'invalid compression specification: unknown compression option "thunk"',
106-
'failure on unknown compression option'
107-
],
108-
[
109-
'gzip:level',
110-
'invalid compression specification: compression option "level" requires a value',
111-
'failure on missing compression level'
112-
],
113-
[
114-
'gzip:level=',
115-
'invalid compression specification: value for compression option "level" must be an integer',
116-
'failure on empty compression level'
117-
],
118-
[
119-
'gzip:level=high',
120-
'invalid compression specification: value for compression option "level" must be an integer',
121-
'failure on non-numeric compression level'
122-
],
123-
[
124-
'gzip:level=236',
125-
'invalid compression specification: compression algorithm "gzip" expects a compression level between 1 and 9',
126-
'failure on out-of-range compression level'
127-
],
128-
[
129-
'gzip:level=9,',
130-
'invalid compression specification: found empty string where a compression option was expected',
131-
'failure on extra, empty compression option'
132-
],
133-
[
134-
'gzip:workers=3',
135-
'invalid compression specification: compression algorithm "gzip" does not accept a worker count',
136-
'failure on worker count for gzip'
137-
],);
138-
formy$cft (@compression_failure_tests)
89+
SKIP:
13990
{
140-
my$cfail =quotemeta($client_fails .$cft->[1]);
141-
my$sfail =quotemeta($server_fails .$cft->[1]);
142-
$node->command_fails_like(
143-
['pg_basebackup','-D',"$tempdir/backup",'--compress',$cft->[0] ],
144-
qr/$cfail/,
145-
'client' .$cft->[2]);
146-
$node->command_fails_like(
91+
skip"postgres was not built with ZLIB support", 6
92+
if (!check_pg_config("#define HAVE_LIBZ 1"));
93+
94+
my$client_fails ='pg_basebackup: error:';
95+
my$server_fails =
96+
'pg_basebackup: error: could not initiate base backup: ERROR:';
97+
my@compression_failure_tests =(
14798
[
148-
'pg_basebackup','-D',
149-
"$tempdir/backup",'--compress',
150-
'server-' .$cft->[0]
99+
'extrasquishy',
100+
'unrecognized compression algorithm "extrasquishy"',
101+
'failure on invalid compression algorithm'
151102
],
152-
qr/$sfail/,
153-
'server' .$cft->[2]);
103+
[
104+
'gzip:',
105+
'invalid compression specification: found empty string where a compression option was expected',
106+
'failure on empty compression options list'
107+
],
108+
[
109+
'gzip:thunk',
110+
'invalid compression specification: unknown compression option "thunk"',
111+
'failure on unknown compression option'
112+
],
113+
[
114+
'gzip:level',
115+
'invalid compression specification: compression option "level" requires a value',
116+
'failure on missing compression level'
117+
],
118+
[
119+
'gzip:level=',
120+
'invalid compression specification: value for compression option "level" must be an integer',
121+
'failure on empty compression level'
122+
],
123+
[
124+
'gzip:level=high',
125+
'invalid compression specification: value for compression option "level" must be an integer',
126+
'failure on non-numeric compression level'
127+
],
128+
[
129+
'gzip:level=236',
130+
'invalid compression specification: compression algorithm "gzip" expects a compression level between 1 and 9',
131+
'failure on out-of-range compression level'
132+
],
133+
[
134+
'gzip:level=9,',
135+
'invalid compression specification: found empty string where a compression option was expected',
136+
'failure on extra, empty compression option'
137+
],
138+
[
139+
'gzip:workers=3',
140+
'invalid compression specification: compression algorithm "gzip" does not accept a worker count',
141+
'failure on worker count for gzip'
142+
],);
143+
formy$cft (@compression_failure_tests)
144+
{
145+
my$cfail =quotemeta($client_fails .$cft->[1]);
146+
my$sfail =quotemeta($server_fails .$cft->[1]);
147+
$node->command_fails_like(
148+
[
149+
'pg_basebackup','-D',
150+
"$tempdir/backup",'--compress',
151+
$cft->[0]
152+
],
153+
qr/$cfail/,
154+
'client' .$cft->[2]);
155+
$node->command_fails_like(
156+
[
157+
'pg_basebackup','-D',
158+
"$tempdir/backup",'--compress',
159+
'server-' .$cft->[0]
160+
],
161+
qr/$sfail/,
162+
'server' .$cft->[2]);
163+
}
154164
}
155165

156166
# Write some files to test that they are not copied.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp