On Thu, Mar 31, 2022 at 11:44:40AM -0400, Tom Lane wrote: > Justin Pryzby <pry...@telsasoft.com> writes: > > Possible responses look like: > > - Use 0 which also means "default" (need to verify that works across > > versions); > > - Or #ifndef ZSTD_CLEVEL_DEFAULT #define ZSTD_CLEVEL_DEFAULT 3; > > - Add a test for a minimum zstd version v1.3.7. This may be a good idea > > for > > v15 in any case, since we're using a few different APIs (at least > > ZSTD_compress and ZSTD_compressStream2 and execve(zstd)). > > In view of 51c0d186d ("Allow parallel zstd compression"), I agree > that some clarity about the minimum supported version of zstd > seems essential. I don't want to be dealing with threading bugs > in ancient zstd versions. However, why do you suggest 1.3.7 in > particular?
That's where I found that ZSTD_CLEVEL_DEFAULT was added, in their git. I've just installed a .deb for 1.3.8, and discovered that the APIs used by basebackup were considered experimental/nonpublic/static-lib-only until 1.4.0 https://github.com/facebook/zstd/releases/tag/v1.4.0 ZSTD_CCtx_setParameter ZSTD_c_compressionLevel ZSTD_c_nbWorkers ZSTD_CCtx_reset ZSTD_reset_session_only ZSTD_compressStream2 ZSTD_e_continue ZSTD_e_end FYI: it looks like parallel, thread workers were also a nonpublic, "experimental" API until v1.3.7. Actually, ZSTD_p_nbThreads was renamed to ZSTD_p_nbWorkers and then (in 1.3.8) renamed to ZSTD_c_nbWorkers. Versions less than 1.3.8 would have required compile-time conditionals for both ZSTD_CLEVEL_DEFAULT and ZSTD_p_nbThreads/ZSTD_p_nbWorkers/ZSTD_c_nbWorkers (but that is moot). Negative compression levels were added in 1.3.4 (but I think the range of their levels was originally -1..-7 and now expanded). And long-distance matching added in 1.3.2. cirrus has installed: linux (debian bullseye) 1.4.8 macos has 1.5.0 freebsd has 1.5.0 debian buster (oldstable) has v1.3.8, which is too old. ubuntu focal LTS has 1.4.4 (bionic LTS has 1.3.3 which seems too old) rhel7 has 1.5.2 in EPEL; SLES 12.5 has zstd 1.3.3, so it won't be supported. But postgres should fail gracefully during ./configure rather than during build. Note that check-world fails if wal_compression is enabled due to two outstanding issues, so it's not currently possible to set that in CI or buildfarm... https://www.postgresql.org/message-id/c86ce84f-dd38-9951-102f-13a931210f52%40dunslane.net
>From 45b94681af7fb822455f5f6114298b030d52e820 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Thu, 31 Mar 2022 17:20:59 -0500 Subject: [PATCH] zstd: require library >=1.4.0 Older versions would fail to compile anyway. --- configure | 22 +++++++++++----------- configure.ac | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/configure b/configure index 8b361e211b8..14d2ca28534 100755 --- a/configure +++ b/configure @@ -9092,19 +9092,19 @@ $as_echo "$with_zstd" >&6; } if test "$with_zstd" = yes; then pkg_failed=no -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for libzstd" >&5 -$as_echo_n "checking for libzstd... " >&6; } +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for libzstd >= 1.4.0" >&5 +$as_echo_n "checking for libzstd >= 1.4.0... " >&6; } if test -n "$ZSTD_CFLAGS"; then pkg_cv_ZSTD_CFLAGS="$ZSTD_CFLAGS" elif test -n "$PKG_CONFIG"; then if test -n "$PKG_CONFIG" && \ - { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libzstd\""; } >&5 - ($PKG_CONFIG --exists --print-errors "libzstd") 2>&5 + { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libzstd >= 1.4.0\""; } >&5 + ($PKG_CONFIG --exists --print-errors "libzstd >= 1.4.0") 2>&5 ac_status=$? $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 test $ac_status = 0; }; then - pkg_cv_ZSTD_CFLAGS=`$PKG_CONFIG --cflags "libzstd" 2>/dev/null` + pkg_cv_ZSTD_CFLAGS=`$PKG_CONFIG --cflags "libzstd >= 1.4.0" 2>/dev/null` test "x$?" != "x0" && pkg_failed=yes else pkg_failed=yes @@ -9116,12 +9116,12 @@ if test -n "$ZSTD_LIBS"; then pkg_cv_ZSTD_LIBS="$ZSTD_LIBS" elif test -n "$PKG_CONFIG"; then if test -n "$PKG_CONFIG" && \ - { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libzstd\""; } >&5 - ($PKG_CONFIG --exists --print-errors "libzstd") 2>&5 + { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libzstd >= 1.4.0\""; } >&5 + ($PKG_CONFIG --exists --print-errors "libzstd >= 1.4.0") 2>&5 ac_status=$? $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 test $ac_status = 0; }; then - pkg_cv_ZSTD_LIBS=`$PKG_CONFIG --libs "libzstd" 2>/dev/null` + pkg_cv_ZSTD_LIBS=`$PKG_CONFIG --libs "libzstd >= 1.4.0" 2>/dev/null` test "x$?" != "x0" && pkg_failed=yes else pkg_failed=yes @@ -9142,14 +9142,14 @@ else _pkg_short_errors_supported=no fi if test $_pkg_short_errors_supported = yes; then - ZSTD_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors --cflags --libs "libzstd" 2>&1` + ZSTD_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors --cflags --libs "libzstd >= 1.4.0" 2>&1` else - ZSTD_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs "libzstd" 2>&1` + ZSTD_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs "libzstd >= 1.4.0" 2>&1` fi # Put the nasty error message in config.log where it belongs echo "$ZSTD_PKG_ERRORS" >&5 - as_fn_error $? "Package requirements (libzstd) were not met: + as_fn_error $? "Package requirements (libzstd >= 1.4.0) were not met: $ZSTD_PKG_ERRORS diff --git a/configure.ac b/configure.ac index 68ade8f7a64..35580988dd0 100644 --- a/configure.ac +++ b/configure.ac @@ -1073,7 +1073,7 @@ AC_MSG_RESULT([$with_zstd]) AC_SUBST(with_zstd) if test "$with_zstd" = yes; then - PKG_CHECK_MODULES(ZSTD, libzstd) + PKG_CHECK_MODULES(ZSTD, libzstd >= 1.4.0) # We only care about -I, -D, and -L switches; # note that -lzstd will be added by AC_CHECK_LIB below. for pgac_option in $ZSTD_CFLAGS; do -- 2.17.1