Fix a few goofs in new backup compression code.

robertmhaas · robertmhaas · commit 61762426e6ed · 2022-03-28T12:19:05.000-04:00
When we try to set the zstd compression level either on the client or on the server, check for errors. For any algorithm, on the client side, don't try to set the compression level unless the user specified one. This was visibly broken for zstd, which managed to set -1 rather than 0 in this case, but tidy up the code for the other methods, too. On the client side, if we fail to create a ZSTD_CCtx, exit after reporting the error. Otherwise we'll dereference a null pointer. Patch by me, reviewed by Dipesh Pandit. Discussion: http://postgr.es/m/CA+TgmoZK3zLQUCGi1h4XZw4jHiAWtcACc+GsdJR1_Mc19jUjXA@mail.gmail.com
diff --git a/src/backend/replication/basebackup_zstd.c b/src/backend/replication/basebackup_zstd.c
@@ -98,13 +98,17 @@ bbsink_zstd_begin_backup(bbsink *sink)
 {
 	bbsink_zstd *mysink = (bbsink_zstd *) sink;
 	size_t		output_buffer_bound;
+	size_t		ret;
 
 	mysink->cctx = ZSTD_createCCtx();
 	if (!mysink->cctx)
 		elog(ERROR, "could not create zstd compression context");
 
-	ZSTD_CCtx_setParameter(mysink->cctx, ZSTD_c_compressionLevel,
-						   mysink->compresslevel);
+	ret = ZSTD_CCtx_setParameter(mysink->cctx, ZSTD_c_compressionLevel,
+								 mysink->compresslevel);
+	if (ZSTD_isError(ret))
+		elog(ERROR, "could not set zstd compression level to %d: %s",
+			 mysink->compresslevel, ZSTD_getErrorName(ret));
 
 	/*
 	 * We need our own buffer, because we're going to pass different data to
diff --git a/src/bin/pg_basebackup/bbstreamer_gzip.c b/src/bin/pg_basebackup/bbstreamer_gzip.c
@@ -116,7 +116,8 @@ bbstreamer_gzip_writer_new(char *pathname, FILE *file,
 		}
 	}
 
-	if (gzsetparams(streamer->gzfile, compress->level,
+	if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) != 0 &&
+		gzsetparams(streamer->gzfile, compress->level,
 					Z_DEFAULT_STRATEGY) != Z_OK)
 	{
 		pg_log_error("could not set compression level %d: %s",
diff --git a/src/bin/pg_basebackup/bbstreamer_lz4.c b/src/bin/pg_basebackup/bbstreamer_lz4.c
@@ -89,7 +89,8 @@ bbstreamer_lz4_compressor_new(bbstreamer *next, bc_specification *compress)
 	prefs = &streamer->prefs;
 	memset(prefs, 0, sizeof(LZ4F_preferences_t));
 	prefs->frameInfo.blockSizeID = LZ4F_max256KB;
-	prefs->compressionLevel = compress->level;
+	if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) != 0)
+		prefs->compressionLevel = compress->level;
 
 	/*
 	 * Find out the compression bound, it specifies the minimum destination
diff --git a/src/bin/pg_basebackup/bbstreamer_zstd.c b/src/bin/pg_basebackup/bbstreamer_zstd.c
@@ -67,6 +67,8 @@ bbstreamer_zstd_compressor_new(bbstreamer *next, bc_specification *compress)
 {
 #ifdef USE_ZSTD
 	bbstreamer_zstd_frame *streamer;
+	int			compresslevel;
+	size_t		ret;
 
 	Assert(next != NULL);
 
@@ -81,11 +83,24 @@ bbstreamer_zstd_compressor_new(bbstreamer *next, bc_specification *compress)
 
 	streamer->cctx = ZSTD_createCCtx();
 	if (!streamer->cctx)
+	{
 		pg_log_error("could not create zstd compression context");
+		exit(1);
+	}
 
 	/* Initialize stream compression preferences */
-	ZSTD_CCtx_setParameter(streamer->cctx, ZSTD_c_compressionLevel,
-						   compress->level);
+	if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) == 0)
+		compresslevel = 0;
+	else
+		compresslevel = compress->level;
+	ret = ZSTD_CCtx_setParameter(streamer->cctx, ZSTD_c_compressionLevel,
+								 compresslevel);
+	if (ZSTD_isError(ret))
+	{
+		pg_log_error("could not set zstd compression level to %d: %s",
+					 compresslevel, ZSTD_getErrorName(ret));
+		exit(1);
+	}
 
 	/* Initialize the ZSTD output buffer. */
 	streamer->zstd_outBuf.dst = streamer->base.bbs_buffer.data;

Original file line number	Diff line number	Diff line change
`@@ -116,7 +116,8 @@ bbstreamer_gzip_writer_new(char pathname, FILE file,`
`116`	`116`	`}`
`117`	`117`	`}`
`118`	`118`
`119`		`- if (gzsetparams(streamer->gzfile, compress->level,`
	`119`	`+ if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) != 0 &&`
	`120`	`+ gzsetparams(streamer->gzfile, compress->level,`
`120`	`121`	`Z_DEFAULT_STRATEGY) != Z_OK)`
`121`	`122`	`{`
`122`	`123`	`pg_log_error("could not set compression level %d: %s",`