Fix a few goofs in new backup compression code.

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
index bb5b668c2ab4c284fc867a3db5f2b67edc554108..5496eaa72b71a37abb9e996344b2517667c5706c 100644 (file)
--- 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
index 1979e956399d0ff27e4e74ea97f53339fbcc87ec..760619fcd7435882f0540eae15195d23f9f7bcb2 100644 (file)
--- 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
index a6ec317e2bd21b453f2d6e6fd13d66fa40144b8f..67f841d96a96f1babaeea30275d3aea8cae10284 100644 (file)
--- 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
index caa5edcaf12308be10717e589f6c72de7e496c23..7946b6350b6d7823cbdc93ddbc33feaa82703db4 100644 (file)
--- 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;