On Fri, Mar 04, 2022 at 04:19:32PM +0900, Michael Paquier wrote:
> On Tue, Feb 22, 2022 at 05:19:48PM -0600, Justin Pryzby wrote:
>
> > As writen, this patch uses zstd level=1 (whereas the ZSTD's default compress
> > level is 6).
>
> Why? ZSTD using this default has its reasons, no? And it would be
> consistent to do the same for ZSTD as for the other two methods.
In my 1-off test, it gets 610/633 = 96% of the benefit at 209/273 = 77% of the
cost.
postgres=# SET wal_compression= 'zstd-6';
postgres=# \set QUIET \\ \timing on \\ SET max_parallel_maintenance_workers=0;
SELECT pg_stat_reset_shared('wal'); begin; CREATE INDEX ON t(a); rollback;
SELECT * FROM pg_stat_wal;
Duración: 2729,017 ms (00:02,729)
wal_bytes | 6102403
postgres=# SET wal_compression= 'zstd-1';
postgres=# \set QUIET \\ \timing on \\ SET max_parallel_maintenance_workers=0;
SELECT pg_stat_reset_shared('wal'); begin; CREATE INDEX ON t(a); rollback;
SELECT * FROM pg_stat_wal;
Duración: 2090,459 ms (00:02,090)
wal_bytes | 6330269
It may be relevant that we're only compressing 8k [0].
It would probably pay off to preserve a compression "dictionary" to be re-used
across FPIs.
> - The supported methods are <literal>pglz</literal> and
> - <literal>lz4</literal> (if <productname>PostgreSQL</productname> was
> - compiled with <option>--with-lz4</option>). The default value is
> - <literal>off</literal>. Only superusers can change this setting.
> + The supported methods are <literal>pglz</literal>, and
> + (if configured when <productname>PostgreSQL</productname>was built)
> + <literal>lz4</literal> and zstd.
> + The default value is <literal>off</literal>.
> + Only superusers can change this setting.
>
> This is making the docs less verbose. I think that you should keep
> the mention to respectively --with-lz4 and --with-zstd after each
> value.
I changed this relative to your latest zstd patch in July since it reads better
to me. YMMV.
On Tue, Feb 22, 2022 at 05:19:48PM -0600, Justin Pryzby wrote:
>> 0001 adds support for zstd
>> 0002 makes more efficient our use of bits in the WAL header
>> 0003 makes it the default compression, to exercise on CI (windows will fail).
>> 0004 adds support for zstd-level
>> 0005-6 are needed to allow recovery tests to pass when wal compression is
>> enabled;
>> 0007 (not included) also adds support for zlib. I'm of the impression nobody
>> cares about this, otherwise it would've been included 5-10 years ago.
> 0003, defaulting to zstd, would require a lot of benchmarking to be
> justified.
Maybe you didn't see that I wrote that it was for CI ?
(In any case, it's impossible to do that without first taking care of 005-6).
> and 0004 to extend compression to support a level
> I have argued against making the code more complicated for such things in the
> past, with reloptions.
I suppose that you're referring to reloptions for toast compression, which was
about controlling LZ4 compression level.
https://www.postgresql.org/message-id/flat/cafitn-vmhu_yakmgno90suih_6ltvmqz5hpqb2itgqtvyoj...@mail.gmail.com
I think you're right that it's not interesting to control the compression level
of LZ4 - but there's no reason to say the same thing of zstd. We're already
debating which is the "right" level to use (1 or 6). I don't think there is a
"right" level - some people will want to trade more CPU for disk writes and
some people won't.
