From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
Cc: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Different compression methods for FPI |
Date: | 2021-03-01 04:57:12 |
Message-ID: | YDx0KDjQEaK+sfeB@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Feb 28, 2021 at 05:08:17PM -0600, Justin Pryzby wrote:
> Does this need to patch ./configure{,.ac} and Solution.pm for HAVE_LIBLZ4 ?
> I suggest to also include an 0002 patch (not for commit) which changes to use a
> non-default compression, to exercise this on the CIs - linux and bsd
> environments now have liblz4 installed, and for windows you can keep "undef".
Good idea. But are you sure that lz4 is available in the CF bot
environments?
> Your patch looks fine, but I wonder if we should first implement a generic
> compression API. Then, the callers don't need to have a bunch of #ifdef.
> If someone calls zs_create() for an algorithm which isn't enabled at compile
> time, it throws the error at a lower level.
Yeah, the patch feels incomplete with its footprint in xloginsert.c
for something that could be available in src/common/ like pglz,
particularly knowing that you will need to have this information
available to frontend tools, no?
> In some cases there's an argument that the compression ID should be globally
> defined constant, not like a dynamic "plugable" OID. That may be true for the
> libpq compression, WAL compression, and pg_dump, since there's separate
> "producer" and "consumer". I think if there's "pluggable" compression (like
> the TOAST patch), then it may have to map between the static or dynamic OIDs
> and the global compression ID.
This declaration should be frontend-aware. As presented, the patch
would enforce zlib or lz4 on top of pglz, but I guess that it would be
more user-friendly to complain when attempting to set up
wal_compression_method (why not just using wal_compression?) to an
unsupported option rather than doing it after-the-fact for the first
full page generated once the new parameter value is loaded.
This stuff could just add tests in src/test/recovery/. See for
example src/test/ssl and with_ssl to see how conditional tests happen
depending on the configure options.
Not much a fan either of assuming that it is just fine to add one byte
to XLogRecordBlockImageHeader for the compression_method field.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | vignesh C | 2021-03-01 05:00:01 | Re: repeated decoding of prepared transactions |
Previous Message | Nitin Jadhav | 2021-03-01 04:52:37 | [PATCH] Bug fix in initdb output |