Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h} - Mailing list pgsql-hackers

From gkokolatos@pm.me
Subject Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}
Date
Msg-id QYtEfeZ2bTdYAeqCR-5rQs_myRhJclxaVfun-OEVSPnauawRqris5UNUTnYtoFy_KnhCbsjtkEeoxvZgLT0UkZTMnMq65BOwklSwFqbvI1U=@pm.me
Whole thread Raw
In response to Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}
List pgsql-hackers

------- 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 word 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 may 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/CABUevEwuq7XXyd4fA0W3jY9MsJu9B2WRbHumAA%2B3WzHrGAQjsg%40mail.gmail.com#b6456fa2adc1cdb049a57bf3587666b9



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: make MaxBackends available in _PG_init
Next
From: Andres Freund
Date:
Subject: Re: Atomic rename feature for Windows.