| 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-10 18:49:48 |
| Message-ID: | 179432185962.3379798.1613728745172918153@burd.me |
| 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.
Thanks again Hannu for giving this your thought and time. I can see
that you are interested/invested in this thread, thanks for that effort.
> 1. Unbounded sample buffer (> 1 GB StringInfo failure + stuck gates)
Fixed.
The evolution sample loop now stops at DICT_MAX_SAMPLES values or a byte
budget (DICT_DEFAULT_SIZE * DICT_SAMPLE_FACTOR for training, plus a
quarter for the held-out set), like the manual trainer. rows[] is in
physical order and either bound can stop the loop early, so it visits
the rows on a stride coprime with numrows (close to the golden ratio),
which keeps a bounded sample spread across the whole table rather than
taking the first few wide rows. Your big_evo repro now trains in ~75 ms
using a few MB. Because the loop no longer throws, the stuck-gates case
is gone too, but the gate restart on a declined or failed candidate
(from v15) covers it regardless.
> 2. Hot Standby recovery guard (and READ ONLY txn semantics)
pg_train_toast_dictionary() now calls
PreventCommandDuringRecovery("pg_train_toast_dictionary()") before the
RELOID lookup, so on a standby it fails with "cannot execute
pg_train_toast_dictionary() during recovery", the same shape as ANALYZE,
instead of the lock error. I left READ ONLY as it is on the primary,
matching ANALYZE and pg_restore_attribute_stats(): both the manual
function and automatic evolution write pg_toast_dictionary in a READ
ONLY transaction, which seems right since ANALYZE does.
> 3. Missing relkind / catalog guard in toast_dictionary_maybe_evolve()
Fixed.
Evolution now returns early unless the relation is a plain table or
matview and not a catalog, the same check dict_check_column() makes for
the manual path, so ANALYZE on a foreign table with an opted-in
dictionary column installs nothing.
I didn't add a regression test, there's no in-tree FDW that returns
sample rows in a core test, and the dummy handler errors before
evolution runs.
> 4. attstorage (STORAGE PLAIN / EXTERNAL) consistency
Fixed with one predicate, dict_attr_uses_dict(), that requires a
dictionary method and a storage mode the toaster compresses (EXTENDED or
MAIN). pg_train_toast_dictionary() and evolution refuse a PLAIN or
EXTERNAL column, and dict_column_uses_dict() / toast_dictionary_keep()
use it too, so SET STORAGE EXTERNAL + VACUUM FULL decompresses the
inline values and retires every dictionary for the column.
compression_zstd_dict has a case for that.
> 5. Rebase note on 4a95f56c04d + minor polish
Done.
The new codecs mirror the hardening: zstd_compress_datum(),
zstd_dict_compress_datum() and zstd_dict_compressed_total() reject a bad
or oversize ZSTD_compressBound() (and lz4_dict_compress_datum() a bad
LZ4_compressBound()) before palloc; the whole-value decompressors assert
AllocSizeIsValid() on the stored size; and the slice decompressors check
the requested length.
Minor polish:
toast_external_info_get() now also maps a plain pointer whose method
bits name something other than pglz or lz4 to the invalid id, so
external and inline validation match; dict_install() uses
CacheInvalidateRelcache(rel) with the relation it already holds; and
alter_table.sgml drops the stale "only n_distinct and
n_distinct_inherited" sentence and notes that per-attribute options are
set on leaf partitions.
v16 is attached, it applies on master as of 952bab8b74d. It has the
AllocSizeIsValid() changes from 4a95f56c04d folded into the new codecs
as you noted.
best.
-greg
| Attachment | Content-Type | Size |
|---|---|---|
| v16-0002-Allow-more-than-four-TOAST-compression-methods.patch | text/x-patch | 38.4 KB |
| v16-0001-Decode-the-compression-method-in-toast_external_.patch | text/x-patch | 6.9 KB |
| v16-0003-Add-zstd-as-a-TOAST-compression-method.patch | text/x-patch | 87.1 KB |
| v16-0005-Add-automatic-dictionary-evolution-during-ANALYZ.patch | text/x-patch | 69.4 KB |
| v16-0004-Add-TOAST-compression-dictionaries-for-zstd-and-.patch | text/x-patch | 151.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zsolt Parragi | 2026-11-10 19:24:43 | Re: BGWORKER_INTERRUPTIBLE workers are never restarted after DROP/ALTER/CREATE DATABASE |
| Previous Message | Osama Abdul Qader | 2026-11-10 18:25:32 | Re: Avoid using raw xmin of frozen tuples in logicalrep conflict detection |