+ * We check for failure here because (1) older versions of the library
+ * do not support ZSTD_c_nbWorkers and (2) the library might want to
+ * reject an unreasonable values (though in practice it does not seem
to do
+ * so).
+ */
+ ret = ZSTD_CCtx_setParameter(mysink->cctx, ZSTD_c_nbWorkers,
+
mysink->workers);
+ if (ZSTD_isError(ret))
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("could not set compression worker count
to %d: %s",
+ mysink->workers,
ZSTD_getErrorName(ret)));
Also because the library may not be compiled with threading. A few days ago, I
tried to rebase the original "parallel workers" patch over the COMPRESS DETAIL
patch but then couldn't test it, even after trying various versions of the zstd
package and trying to compile it locally. I'll try again soon...
I think you should also test the return value when setting the compress level.
Not only because it's generally a good idea, but also because I suggested to
support negative compression levels. Which weren't allowed before v1.3.4, and
then the range is only defined since 1.3.6 (ZSTD_minCLevel). At some point,
the range may have been -7..22 but now it's -131072..22.
lib/compress/zstd_compress.c:int ZSTD_minCLevel(void) { return
(int)-ZSTD_TARGETLENGTH_MAX; }
lib/zstd.h:#define ZSTD_TARGETLENGTH_MAX ZSTD_BLOCKSIZE_MAX
lib/zstd.h:#define ZSTD_BLOCKSIZE_MAX (1<<ZSTD_BLOCKSIZELOG_MAX)
lib/zstd.h:#define ZSTD_BLOCKSIZELOG_MAX 17
; -1<<17
-131072
>From 80f45cfbe13d6fc0f16e49b7ea76f1e50afb632c Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Thu, 10 Mar 2022 20:16:19 -0600
Subject: [PATCH] pg_basebackup: support Zstd negative compression levels
"higher than maximum" is bogus
TODO: each compression methods should enforce its own levels
---
src/backend/replication/basebackup_zstd.c | 7 +++++--
src/bin/pg_basebackup/bbstreamer_zstd.c | 5 ++++-
src/common/backup_compression.c | 6 +++++-
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/backend/replication/basebackup_zstd.c b/src/backend/replication/basebackup_zstd.c
index 4835aa70fca..74681ee3fe8 100644
--- a/src/backend/replication/basebackup_zstd.c
+++ b/src/backend/replication/basebackup_zstd.c
@@ -79,7 +79,7 @@ bbsink_zstd_new(bbsink *next, bc_specification *compress)
else
{
compresslevel = compress->level;
- Assert(compresslevel >= 1 && compresslevel <= 22);
+ Assert(compresslevel >= -7 && compresslevel <= 22 && compresslevel != 0);
}
sink = palloc0(sizeof(bbsink_zstd));
@@ -108,8 +108,11 @@ bbsink_zstd_begin_backup(bbsink *sink)
if (!mysink->cctx)
elog(ERROR, "could not create zstd compression context");
- ZSTD_CCtx_setParameter(mysink->cctx, ZSTD_c_compressionLevel,
+ ret = ZSTD_CCtx_setParameter(mysink->cctx, ZSTD_c_compressionLevel,
mysink->compresslevel);
+ if (ZSTD_isError(ret))
+ elog(ERROR, "could not create zstd compression context: %s",
+ ZSTD_getErrorName(ret));
/*
* We check for failure here because (1) older versions of the library
diff --git a/src/bin/pg_basebackup/bbstreamer_zstd.c b/src/bin/pg_basebackup/bbstreamer_zstd.c
index e17dfb6bd54..640729003a4 100644
--- a/src/bin/pg_basebackup/bbstreamer_zstd.c
+++ b/src/bin/pg_basebackup/bbstreamer_zstd.c
@@ -85,8 +85,11 @@ bbstreamer_zstd_compressor_new(bbstreamer *next, bc_specification *compress)
pg_log_error("could not create zstd compression context");
/* Initialize stream compression preferences */
- ZSTD_CCtx_setParameter(streamer->cctx, ZSTD_c_compressionLevel,
+ ret = ZSTD_CCtx_setParameter(streamer->cctx, ZSTD_c_compressionLevel,
compress->level);
+ if (ZSTD_isError(ret))
+ pg_log_error("could not set compression level to: %d: %s",
+ compress->level, ZSTD_getErrorName(ret));
/*
* We check for failure here because (1) older versions of the library
diff --git a/src/common/backup_compression.c b/src/common/backup_compression.c
index 969e08cca20..c0eff30024c 100644
--- a/src/common/backup_compression.c
+++ b/src/common/backup_compression.c
@@ -260,13 +260,17 @@ validate_bc_specification(bc_specification *spec)
else if (spec->algorithm == BACKUP_COMPRESSION_LZ4)
max_level = 12;
else if (spec->algorithm == BACKUP_COMPRESSION_ZSTD)
+ {
max_level = 22;
+ /* The minimum level depends on the version.. */
+ min_level = -7;
+ }
else
return psprintf(_("compression algorithm \"%s\" does not accept a compression level"),
get_bc_algorithm_name(spec->algorithm));
if (spec->level < min_level || spec->level > max_level)
- return psprintf(_("compression algorithm \"%s\" expects a compression level between %d and %d"),
+ return psprintf(_("compression algorithm \"%s\" expects a nonzero compression level between %d and %d"),
get_bc_algorithm_name(spec->algorithm),
min_level, max_level);
}
--
2.17.1
