Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}

From: gkokolatos(at)pm(dot)me
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}
Date: 2022-04-13 14:58:28
Message-ID: QYtEfeZ2bTdYAeqCR-5rQs_myRhJclxaVfun-OEVSPnauawRqris5UNUTnYtoFy_KnhCbsjtkEeoxvZgLT0UkZTMnMq65BOwklSwFqbvI1U=@pm.me
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

------- Original Message -------
On Wednesday, April 13th, 2022 at 7:25 AM, Michael Paquier <michael(at)paquier(dot)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(at)pm(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2022-04-13 15:03:06 Re: Atomic rename feature for Windows.
Previous Message Tom Lane 2022-04-13 14:43:15 Re: make MaxBackends available in _PG_init