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

From: Greg Burd <greg(at)burd(dot)me>
To: Manu <manuelreyesbravo(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, 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-10-27 11:47:13
Message-ID: ormEJV9zReviaUUzJw6tZ2RSvu5Up688drMbhZWYk4jn4VshCUL9L1UNnZVadUPZqkX7FS7mtv3wzoJ50Frnd1DGTyDmTPe1JMjYpasdum0=@burd.me
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sunday, October 25th, 2026 at 9:21 PM, Manu <manuelreyesbravo(at)gmail(dot)com> wrote:

> Hi Greg,

Hi Manu,

Thanks for the depth here. All four diffs are in v12, which I'll post as
its own message; replies inline.

> 1. lz4 without zstd does not compile. lz4_dict_get() is only under
> #ifdef USE_LZ4 and calls dict_fetch(), which is static and inside
> #ifdef USE_ZSTD; the call is at toast_dictionary.c:537.

Fixed as you had it: dict_fetch() now sits under USE_ZSTD || USE_LZ4 and
only the zstd invalidation-callback registration stays under USE_ZSTD. I
build all three configurations now (zstd+lz4, lz4-only, zstd-only), and
each passes make check (243). cfbot never caught this because it builds
with both; I should have built the single-codec configs myself.

> 2. With zstd but without lz4, compression_zstd_dict fails: its SET
> COMPRESSION lz4 gets "compression method lz4 not supported". The same
> diff uses pglz there.

Taken; the test uses pglz for the switch-away-from-zstd_dict case, which
does not depend on lz4 being built.

> 3. 0005 swallows errors, a cancel included. [...] A real SIGINT on entry
> to it: ANALYZE completed and only logged "... was skipped: canceling
> statement due to user request".

A swallowed cancel and the half-done dict_install() is fixed. v12 runs the
evaluation in an internal subtransaction and re-throws, exactly as your
diff does: BeginInternalSubTransaction, the work, then either
ReleaseCurrentSubTransaction on success or RollbackAndReleaseCurrent on
error; and it re-throws when edata->sqlerrcode is ERRCODE_QUERY_CANCELED
or an interrupt is still pending, since ProcessInterrupts() clears
QueryCancelPending before raising. A failed step now rolls back its TOAST
scan and buffer pins instead of leaking, and a mid-dict_install() error no
longer leaves the old dictionary non-current with no replacement. Under
cassert+ASan a SIGINT gives the normal ERROR, a damaged chunk gives one
WARNING and no leak, and adoption still works in plain ANALYZE, in
BEGIN/COMMIT, BEGIN/ROLLBACK, a rolled-back savepoint, and VACUUM
(ANALYZE).

> 4. The dictionary caches hold one entry. Every switch between
> dictionaries fetches the dictionary again, and zstd digests it again.
> [...] one column retrained without a rewrite, which evolution now does
> on its own [...] 258,163 (1,946 fetches for 2,000 values).

Right, and evolution makes this the common case, not a corner one. v12
takes your 4-entry LRU keyed by OID for both directions, picking the
victim after dict_fetch() since the fetch can run the invalidation
callback and empty the cache. The instruction counts match what you saw:
the two-dictionary scan drops from ~265k to ~28k per value, and the
retrain-without-rewrite case from ~258k to ~28k. I kept your 4; it is
arbitrary but it covers a scan that alternates between an old and a new
dictionary with headroom, and the per-backend memory is bounded.

> 5. Evolution without drift: [...] At 6,000 one evaluation calls
> ZSTD_createCDict() 12,005 times [...] The fourth diff replaces
> compressed_size() with compressed_total(), one call per dictionary.

Taken. compressed_total() digests each dictionary once and loops the
held-out values inside, so an evaluation now builds two CDicts instead of
one per value. Same totals, same decision, and the held-out test stops
dominating the ANALYZE it rides on.

> 6. On the ~1kB numbers: a 1 kB value alone in its row stays below
> TOAST_TUPLE_THRESHOLD (heapam.c:2266), so it is stored uncompressed with
> every method [...]. At 3 kB the dictionary takes lz4 from 1.94 to 2.50
> on docs, 3.58 to 4.76 on json.

You're right, a ~1 kB value never reaches the compressor, so the "+80%
at 1 kB" number was measuring nothing on the TOAST path; it came from a
standalone CLI run, not from values actually stored. The dictionary
helps once a value crosses TOAST_TUPLE_THRESHOLD.

> Corrupt dictionary values error cleanly under ASan, and substr() matches
> the raw text at 56 offset/length pairs on UTF8 and SQL_ASCII.

Matches my runs too, corrupt values raise the data-corrupted error with no
sanitizer report, and substr() over the offset/length grid matches the raw
text on both encodings.

Thanks again; the subtransaction and the cache fixes in particular are
ones the series needed. v12 follows.

best.

-greg

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2026-10-27 11:50:16 Re: Fix rescan costing of single-batch hash joins
Previous Message Hayato Kuroda (Fujitsu) 2026-10-27 11:40:25 RE: Use WALReadFromBuffers in more places