Re: ZSTD TOAST compression, and an extensible compression method encoding

From: Greg Burd <greg(at)burd(dot)me>
To: Hannu Krosing <hannuk(at)google(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Manu <manuelreyesbravo(at)gmail(dot)com>, Nikhil Kumar Veldanda <veldanda(dot)nikhilkumar17(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Japin Li <japinli(at)hotmail(dot)com>, Dharin Shah <dharinshah95(at)gmail(dot)com>
Subject: Re: ZSTD TOAST compression, and an extensible compression method encoding
Date: 2026-11-14 13:50:49
Message-ID: avhZ9BHes7D6yc5Q@werk
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Nov 9, 2026 at 17:54 UTC, Hannu Krosing <hannuk(at)google(dot)com> wrote:
> Thanks! I ran another pass on v15 under cassert + UBSan, 002_pg_upgrade,
> amcheck, and a streaming Hot Standby.

v17 is attached; it applies on master as of 77bba5454f9. There was no
new review of v16, so this round comes from a full pass of my own over
the series, held to the standard I'd expect a committer to apply.

Most of the changes are in 0001 and 0002, since those are the ones
closest to being committed.

0001 (toast_external_info_get)

Michael's objection to v7-0001 [1] was that adding only compress_method
to toast_external_data left it "half-baked due to the inconsistency this
brings with extsize and VARATT_EXTINFO_GET_EXTSIZE", and that
is_compressed belonged there too.

v16 still had that "half-baked" idea in code. Every caller derived the
saved size and the compressed flag from the raw extinfo word. v17's
0001 decodes all three (extsize, is_compressed, compress_method) once
and removes extinfo from the struct. The twelve places in detoast.c,
toast_compression.c, reorderbuffer.c and amcheck that computed the size
or the flag from extinfo now read the fields, and amcheck's
ToastedAttribute keeps the saved size rather than the raw word.
toast_save_datum() still builds va_extinfo itself. No behavior change.

0002 (long form)

With 0001 normalizing the struct, "invalid method" and "not compressed"
are now distinct (!is_compressed vs. is_compressed && compress_method ==
INVALID), so callers no longer need the raw word to tell them apart.

amcheck printed the folded method ID: a forged pointer whose method byte
was 0 or 200 was reported as "invalid compression method id 3".
toast_external_data now also keeps the stored method bits and method
byte, and amcheck reports what is on disk, distinguishing a plain
pointer with bad method bits, a long pointer without the long-form bits,
and a long pointer whose byte is not a long-form method. The validation
that the inline header and the pointer share is now one function,
toast_compression_id_resolve(), instead of two copies. I dropped
VARTAG_IS_ONDISK_OID8(): it had two callers, and you have said there are
already too many of these helpers.

There was no in-tree test of any of the new rejection paths; the
corruption checks I described earlier lived in a scratch harness. A new
TAP test, amcheck/t/007_verify_toast_pointer.pl, forges pointers on disk
and checks that amcheck reports each case with the stored value and that
reading them fails cleanly. 0002 has the plain-pointer cases; 0003 adds
the long-pointer ones once zstd writes long-form pointers. The commit
message also says plainly that 0002's writer side is first reached by
zstd in 0003.

0003 (zstd)

The slice decoder sizes its window buffer from the frame header, and
that buffer lives in the backend-lifetime decompression context, so a
corrupt header could grow the context to zstd's default limit of 128MB
outside any memory context. The context now caps the window at 2^23
(the default level never uses more than 2^21 in libzstd 1.4.0 through
1.5.7), and the slice path checks the frame's recorded content size
against our header, as the whole-value path already does through
ZSTD_decompressDCtx().

The slice "loop" made one ZSTD_decompressStream() call, since all the
input was handed over at once, so its CHECK_FOR_INTERRUPTS() did
nothing. It now feeds the input in ZSTD_DStreamInSize() chunks (four
calls for a 1.5MB slice under gdb). And the text slice tests did not
reach the slice path in a UTF-8 database, because text_substring() asks
for four bytes per character; the test now slices a bytea across the
128kB block boundary. The comment and commit message had overstated the
cost of a one-shot ZSTD_compress(): it allocates about 90kB for a 4kB
value, and about 1.3MB only from 1MB up.

0004 (dictionaries)

- REPLICA IDENTITY FULL logged old tuples with dictionary values
decompressed, because HEAP_HASEXTERNAL sends the tuple through
toast_flatten_tuple(). In a 2000-row UPDATE that was 10MB of WAL
instead of 0.7MB. Logical decoding reads those values with the
catalog as of the change, which still has their dictionary, so a new
toast_flatten_tuple_for_wal() fetches only out-of-line fields there.
test_decoding still decodes the old values after the dictionary has
been retired.
- pg_train_toast_dictionary() required ownership while ANALYZE, which
installs dictionaries through 0005, accepts MAINTAIN. It now requires
MAINTAIN too, checked before the lock as before.
- A table owner had no way to see their own dictionaries, since the
catalog is closed to PUBLIC. A new view, pg_toast_dictionaries, shows
their metadata (not their contents) for the columns the user can
read, as pg_stats does for pg_statistic.
- pg_dump --binary-upgrade ran one dictionary query per table. It now
collects the tables that have dictionaries up front.
- The docs now say that plain dumps and logical replication carry no
dictionaries.

0005 (evolution)

Several things here were wrong or undocumented.

- Autovacuum's ANALYZE trains and installs dictionaries. I think that
is what a column that opts in wants, but it was documented nowhere.
It is now, and a new SIGHUP setting, dictionary_evolution_autovacuum,
turns it off.
- ANALYZE in a READ ONLY transaction installed a dictionary. That
matches what I wrote last time about following ANALYZE, but ANALYZE
is allowed there because its statistics don't change what a dump
contains, and a new dictionary changes how every later value is
stored. Evolution now does nothing in a read-only transaction. The
manual function is unchanged, since it is an explicit request.
- dictionary_evolution_min_interval silently saturated at 24.8 days; it
now uses TimestampDifferenceExceedsSeconds().
- The sampling loop now honors the cost-based vacuum delay, and skips
values over 1MB: only pglz can decompress a prefix, so the "128kB
prefix" claim in the commit message was wrong for zstd and lz4.
- Hitting the retained-dictionary cap logged the same line on every
ANALYZE; it now restarts the gates, and the message follows the style
guide (errdetail/errhint, qualified name). The WARNING for a skipped
round keeps the original error code.
- TRUNCATE kept the old row-count baseline, so evolution stayed off
until the table regrew past it. TRUNCATE now resets it.
- The regression test claimed the first rows "decode with the retired
dictionary stamped into them", but they were plain zstd. The test
now rewrites them under the first dictionary and checks they read
back after a second is adopted and that VACUUM FULL then drops the
first. It also covers TRUNCATE and read-only transactions.
- The settings moved to the vacuum group; tab completion knows the
column options.

Cost when the feature is not used

Robert's question from the first thread [2] was what this costs users
who don't use it. I ran release builds (no cassert) of the series base
and of v17 on an m5d.metal (Xeon 8259CL, turbo and deep C-states off,
THP never), each server pinned to its own eight cores on one NUMA node,
15 runs per cell in ABBA order, with an A/A run of the base against
itself first:

workload base A/A v17/base v17/base
ms (cores A) (swapped)
INSERT..SELECT, 400k lz4 values 269 1.088 0.985 0.980
INSERT..SELECT, 100k pglz values 104 1.078 0.996 1.018
INSERT, 2M narrow rows 2969 1.070 1.034 0.971
UPDATE, 400k rows with lz4 687 1.025 1.009 1.022
scan + detoast, 400k lz4 values 1777 1.003 1.000 1.120*
scan + detoast, 2000 external 169 0.929 0.927 0.998
substr(), 2000 external 44 1.005 0.955 0.954

The A/A column is the noise floor: the base against itself, on
different cores, already moves up to 9%. Every v17/base ratio is inside
that. The one that holds on both core assignments is substr() of
external values, about 4.5% faster; I have not looked into why.

The 1.120 marked * came from a single run of that server; with both
builds on the same cores, one at a time and 24 runs each, that workload
measured 0.999 and 1.000.

So for users who use neither zstd nor dictionaries the series costs
nothing measurable here, which is what the code suggests. The only new
per-value work on these paths is a test of the compressed header's
method bits in heap_fill_tuple(), reached only for values already
compressed in-line.

best.

-greg

[1] https://postgr.es/m/aryp-_pANFiyzVlx@paquier.xyz
[2] https://postgr.es/m/CA+TgmoY8uCKFvVYDQsLbfhY3JYrduXE7WHi75hmfTWxjWioaaw@mail.gmail.com

Attachment Content-Type Size
v17-0005-Add-automatic-dictionary-evolution-during-ANALYZ.patch text/plain 87.8 KB
v17-0001-Decode-TOAST-pointers-fully-in-toast_external_in.patch text/plain 12.9 KB
v17-0002-Allow-more-than-four-TOAST-compression-methods.patch text/plain 46.8 KB
v17-0003-Add-zstd-as-a-TOAST-compression-method.patch text/plain 94.8 KB
v17-0004-Add-TOAST-compression-dictionaries-for-zstd-and-.patch text/plain 168.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Lakhin 2026-11-14 14:00:01 Re: 041_checkpoint_at_promote.pl might fail due to race condition on child kill
Previous Message Greg Burd 2026-11-14 13:14:21 Re: Let an ordering index scan hand its ORDER BY value to the target list