On 15/06/2021 06:42, Justin Pryzby wrote:
> On Tue, Jun 15, 2021 at 11:39:24AM +0900, Michael Paquier wrote:
>> On Mon, Jun 14, 2021 at 08:42:08PM -0500, Justin Pryzby wrote:
>>>>> It's USERSET following your own suggestion (which is a good suggestion):
>>>> + {"wal_compression_method", PGC_SIGHUP, WAL_SETTINGS,
>>>> + gettext_noop("Set the method used to compress full page images in the WAL."),
>>>> + NULL
>>>> + },
>>>> + &wal_compression_method,
>>>> + WAL_COMPRESSION_PGLZ, wal_compression_options,
>>>> + NULL, NULL, NULL
>>>> Any reason to not make that user-settable? If you merge that with
>>>> wal_compression, that's not an issue.
>>
>> Hmm, yeah. This can be read as using PGC_USERSET. With the second
>> part of my sentence, I think that I imply to use PGC_SUSET and be
>> consistent with wal_compression, but I don't recall my mood from one
>> month ago :) Sorry for the confusion.
>
> Hold on - we're all confused (and I'm to blame). The patch is changing the
> existing wal_compression GUC, rather than adding wal_compression_method.
> It's still SUSET, but in earlier messages, I called it USERSET, twice.
See prior discussion on the security aspect:
https://www.postgresql.org/message-id/55269915.1000309%40iki.fi. Adding
different compression algorithms doesn't change anything from a security
point of view AFAICS.
>>> The goal of the patch is to give options, and the overhead of adding both zlib
>>> and lz4 is low. zlib gives good compression at some CPUcost and may be
>>> preferable for (some) DWs, and lz4 is almost certainly better (than pglz) for
>>> OLTPs.
>>
>> Anything will be better than pglz. I am rather confident in that.
>> What I am wondering is if we need to eat more bits than necessary for
>> the WAL record format, because we will end up supporting it until the
>> end of times.
>
> Why ? This is WAL, not table data. WAL depends on the major version, so
> I think wal_compression could provide a different set of compression methods at
> every major release?
>
> Actually, I was just thinking that default yes/no/on/off stuff maybe should be
> defined to mean "lz4" rather than meaning pglz for "backwards compatibility".
+1
- Heikki