| From: | Manu <manuelreyesbravo(at)gmail(dot)com> |
|---|---|
| To: | Greg Burd <greg(at)burd(dot)me> |
| 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-26 01:21:37 |
| Message-ID: | 179297769702.742704.7777764998846128300@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Greg,
On Sun, Oct 25, 2026 at 4:33 PM Greg Burd <greg(at)burd(dot)me> wrote:
> Hello, here is v11.
On the question you put up front: I think evolution belongs in,
opt-in and off by default as you have it; what follows is about how,
not whether, and point 3 is the one I'd fix first. On your question
in the previous mail, I'd leave the 0003 message as it is; it already
names the case.
v11 applies cleanly on master dfd7ede529. With zstd 1.5.7 and lz4
1.10.0, make check (243 tests) and amcheck pass at -O2 and under
cassert+ASan, with no warnings. The four attached diffs stack in the
order numbered below.
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. The first diff
puts dict_fetch() under USE_ZSTD || USE_LZ4 and keeps only the zstd
callback registration under USE_ZSTD. An lz4-only build then passes
make check, and it reads and writes lz4_dict values whose dictionary a
zstd build trained.
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.
3. 0005 swallows errors, a cancel included.
toast_dictionary_maybe_evolve() catches every ERROR with
FlushErrorState() and no subtransaction (toast_dictionary.c:1112-1290).
A real SIGINT on entry to it: ANALYZE completed and only logged "...
was skipped: canceling statement due to user request". A sampled
external value with a damaged chunk number, which errors inside the
TOAST index scan: ANALYZE returns with seven "resource was not closed"
warnings. From reading, an error halfway through dict_install() would
also commit the old dictionary as non-current with no new one.
The third diff runs the evaluation in an internal subtransaction and
re-throws a cancel. It tests the error code, because ProcessInterrupts()
clears QueryCancelPending before raising it; testing only the pending
flags still swallows the SIGINT. With it, under cassert+ASan, the
SIGINT gives the usual ERROR, the damaged chunk one WARNING and no
leak, and adoption works in plain ANALYZE, 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.
In user-space instructions per value: one zstd_dict column 23,223,
two in a row 265,236 (4,000 fetches for 4,000 values); lz4 11,501 and
189,002. One column retrained without a rewrite, which evolution now
does on its own, with new rows in space freed among old ones: 258,163
(1,946 fetches for 2,000 values).
The second diff, on top of the first, gives each cache 4 entries keyed
by OID, least recently used out. The victim is picked after
dict_fetch(), because dict_fetch() can run the invalidation callback.
With it, those cases take 2 fetches and 27,872, 10,836 and 27,891
instructions per value; one dictionary stays at 23,223. The 4 is
arbitrary, and lz4 can now keep up to 4 dictionary copies per backend,
each up to 1 MB. Under ASan it stays correct with retrains, VACUUM
FULL and DROP COLUMN from another session between one backend's
reads, eviction across six columns and debug_discard_caches = 1.
5. Evolution without drift: I ran 24 ANALYZEs on unchanged data with a
dictionary trained on the same distribution, setting min_gain to 1.0
so the incumbent stayed. The gains were 0.9% to 2.3% at 600 and at
6,000 held-out values, all under the default 5% margin. At 6,000 one
evaluation calls ZSTD_createCDict() 12,005 times: 12,000 from the
held-out test, one digest per size it measures, and 5 inside ZDICT.
The fourth diff replaces compressed_size() with compressed_total(),
one call per dictionary: two digests, same totals and decision.
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, even with toast_tuple_target = 128. At 3 kB the
dictionary takes lz4 from 1.94 to 2.50 on docs, 3.58 to 4.76 on json.
Corrupt dictionary values error cleanly under ASan, and substr()
matches the raw text at 56 offset/length pairs on UTF8 and SQL_ASCII.
Numbers: measurements-v11.txt; scripts: tests-v11.txt.
Regards,
Manu
| Attachment | Content-Type | Size |
|---|---|---|
| nocfbot-v11-lz4-without-zstd.diff.txt | text/plain | 3.0 KB |
| nocfbot-v11-dict-cache-on-fix.diff.txt | text/plain | 7.0 KB |
| nocfbot-v11-evolve-subxact.diff.txt | text/plain | 4.0 KB |
| nocfbot-v11-evolve-digest-once.diff.txt | text/plain | 8.0 KB |
| measurements-v11.txt | text/plain | 7.8 KB |
| tests-v11.txt | text/plain | 45.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Paul Kim | 2026-10-26 01:34:58 | Re: Be strict when request to flush past end of WAL in WaitXLogInsertionsToFinish |
| Previous Message | Richard Guo | 2026-10-26 01:18:37 | Re: Wrong results from a parameterized Append |