Date: Wed, 13 Apr 2022 14:58:28 +0000
To: Michael Paquier <michael@paquier.xyz>
From: gkokolatos@pm.me
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>,
 Robert Haas <robertmhaas@gmail.com>
Reply-To: gkokolatos@pm.me
Subject: Re: Fixes for compression options of pg_receivewal and refactoring of
 backup_compression.{c,h}
Message-ID: 
 <QYtEfeZ2bTdYAeqCR-5rQs_myRhJclxaVfun-OEVSPnauawRqris5UNUTnYtoFy_KnhCbsjtkEeoxvZgLT0UkZTMnMq65BOwklSwFqbvI1U=@pm.me>
In-Reply-To: <YlZernJnRsqntRGC@paquier.xyz>
References: <YlPQGNAAa04raObK@paquier.xyz>
 <Pw7ltgmMRlWiJ8w3EW_1Paoeaf1QIJtV-c-U9UgF8f12w3wxpFbqwp97hr3OchnlOFetTA50zYcMQ3coWDutqP3a1qsG2SXOKipKY3lNqBE=@pm.me>
 <YlVE6E687KDOABoN@paquier.xyz> <YlZernJnRsqntRGC@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/QYtEfeZ2bTdYAeqCR-5rQs_myRhJclxaVfun-OEVSPnauawRqris5UNUTnYtoFy_KnhCbsjtkEeoxvZgLT0UkZTMnMq65BOwklSwFqbvI1U%3D%40pm.me>
Precedence: bulk


------- Original Message -------
On Wednesday, April 13th, 2022 at 7:25 AM, Michael Paquier <michael@paquier=
.xyz> wrote:


>
>
> On Tue, Apr 12, 2022 at 06:22:48PM +0900, Michael Paquier wrote:
>
> > On Mon, Apr 11, 2022 at 12:46:02PM +0000, gkokolatos@pm.me wrote:
> >
> > > It looks good. If you choose to discard the comment regarding the use=
 of
> > > 'method' over 'algorithm' from above, can you please use the full wor=
d in the
> > > variable, e.g. 'wal_compress_algorithm' instead of 'wal_compress_algo=
'. I can
> > > not really explain it, the later reads a bit rude. Then again that ma=
y be just
> > > me.
> >
> > Thanks. I have been able to do an extra pass on 0001 and 0002, fixing
> > those naming inconsistencies with "algo" vs "algorithm" that you and
> > Robert have reported, and applied them. For 0003, I'll look at it
> > later. Attached is a rebase with improvements about the variable
> > names.
>
> This has been done with the proper renames. With that in place, I see
> no reason now to not be able to set the compression level as it is
> possible to pass it down with the options available. This requires
> only a couple of lines, as of the attached. LZ4 has a dummy structure
> called LZ4F_INIT_PREFERENCES to initialize LZ4F_preferences_t, that
> holds the compression level before passing it down to
> LZ4F_compressBegin(), but that's available only in v1.8.3. Using it
> risks lowering down the minimal version of LZ4 we are able to use now,
> but replacing that with a set of memset()s is also a way to set up
> things as per its documentation.
>
> Thoughts?

It's really not hard to add compression level. However we had briefly
discussed it in the original thread [1] and decided against. That is why
I did not write that code. If the community thinks differently now, let
me know if you would like me to offer a patch for it.

Cheers,
//Georgios


[1] https://www.postgresql.org/message-id/flat/CABUevEwuq7XXyd4fA0W3jY9MsJu=
9B2WRbHumAA%2B3WzHrGAQjsg%40mail.gmail.com#b6456fa2adc1cdb049a57bf3587666b9