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


Reply via email to