Date: Wed, 21 Sep 2022 22:37:16 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@lists.postgresql.org
Subject: Re: pg_basebackup's --gzip switch misbehaves
Message-ID: <20220922033716.GL31833@telsasoft.com>
References: <1400032.1662217889@sss.pgh.pa.us>
 <YxQ1tIYF8hbrNO7q@paquier.xyz>
 <YyAtkOU/C/SD9qlL@paquier.xyz>
 <20220922003148.GK31833@telsasoft.com>
 <Yyu5d/wUcEaLJje5@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yyu5d/wUcEaLJje5@paquier.xyz>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20220922033716.GL31833%40telsasoft.com>
Precedence: bulk

On Thu, Sep 22, 2022 at 10:25:11AM +0900, Michael Paquier wrote:
> On Wed, Sep 21, 2022 at 07:31:48PM -0500, Justin Pryzby wrote:
> > I think at some point (maybe before releasing 1.3.4) the range was
> > increased to very large(small), negative levels.  It's possible to query
> > the library about the lowest supported compression level, but then
> > there's a complication regarding the client-side library version vs the
> > server-side version.  So it seems better to just use -7.
> 
> Indeed.  Contrary to the default level, there are no variables for the
> minimum and maximum levels.  As you are pointing out, a lookup at 
> zstd_compress.c shows that we have ZSTD_minCLevel() and
> ZSTD_maxCLevel() that assign the bounds.  Both are available since
> 1.4.0.  We still need a backend-side check as the level passed with a
> BASE_BACKUP command would be only validated there.  It seems to me 
> that this is going to be less of a headache in the long-term if we
> just use those routines at runtime, as zstd wants to keep some freedom
> with the min and max bounds for the compression level, at least that's
> the flexibility that this gives the library.  So I would tweak things
> as the attached.

Okay.  Will that complicate tests at all?  It looks like it's not an
issue for the tests currently proposed in the CF APP.
https://commitfest.postgresql.org/39/3835/

However the patch ends up, +0.75 to backpatch it to v15 rather than
calling it a new feature in v16.

-- 
Justin