Hi hackers! This is v7, and it is now three patches. The thread had grown into one patch doing several things at once, so I split it, and the first of the three is useful on its own.
Sorry for the long silence, and for leaving several review comments
unanswered since March. I have tried to answer all of them below.
---- 0001: reuse a zstd compression context ----
This one is independent of the rest of the thread, so I am putting it
first as low-hanging fruit.
XLogCompressBackupBlock() calls ZSTD_compress(), which creates and
destroys a ZSTD_CCtx on every call. That context is not small: at the
default level ZSTD_estimateCCtxSize() reports 1.3MB. So a backend
running with wal_compression = zstd pays that allocation once per
full-page image, and a record can carry up to XLR_MAX_BLOCK_ID + 1 of
them.
The patch creates the context on first use and keeps it for the life of
the backend, compressing with ZSTD_compressCCtx(). On a statement that
logs 48k full-page images this is about 12% faster.
Pros: small, one file, no format change, and the WAL it produces is
identical byte for byte.
Cons: a backend that used zstd once holds the 1.3MB until it exits. I
think that is the right trade, but it is a real change in footprint for
a backend that compresses once and never again.
---- 0002: whole-record compression alongside FPI compression ----
This is the feature this thread started with, with Fujii-san's
suggestion applied: it complements per-FPI compression instead of
replacing it.
When a record is larger than wal_compression_threshold, it is compressed
as a single unit instead of compressing each full-page image on its own.
That wins whenever the images in one record share content, which is
typical of B-tree index builds. On CREATE INDEX over 10M random floats,
WAL drops from 160MB to 132MB with lz4 and from 125MB to 97MB with zstd.
Only lz4 and zstd take part now. pglz refuses input it cann't shrink
significantly, which a record made of full-page images rarely clears, so
in the earlier versions it cost a flatten and a compression attempt and
saved nothing measurable. It still compresses full-page images exactly as
before.
Pros: it reaches a redundancy that per-FPI compression cannot see, and
records below the threshold are untouched. Every record still decodes
on its own, so nothing about how WAL is read changes. Setting the
threshold above the largest possible record restores today's behaviour.
Cons:
1. Memory. The per-block compressed_page arrays are replaced by two
buffers carved out in one piece, one staging the record and one
taking the compressor output. Each is 274300 bytes with BLCKSZ 8192
and zstd compiled in, against 273372 bytes for the old arrays at
their 33-block maximum. So a backend with compression enabled holds
about twice what it held before. The staging buffer reproduces the
old allocation; the output buffer is genuinely new. It cannot be
removed without compressing straight off the rdt chain, and the
compressed length has to be known before WAL space is reserved, so
the output has to be materialized somewhere. Perhaps, I can just
assume the buffer of a twice smaller size, or somthing like that...
2. A low threshold can make WAL bigger. Whole-record compression
displaces per-FPI compression for the records it takes, and adds 8
bytes of header to each. On a pgbench run, threshold 32 produced 720
bytes of WAL per transaction against 622 at the default 512.
---- 0003: compress records against earlier records (WIP) ----
This is the direction I mentioned in the first message of this thread,
and it is not in a good shape yet. Just a prototype, it womewhat works but
the design discussion is needed.
Half the WAL a pgbench run produces is out of reach for per-record
compression. On a 1.6GB sample the median record was 72 bytes, and the
0.8% of records over 512 bytes held 47.5% of the bytes. A 72-byte
record does not compress on its own. It compresses very well against
the records before it.
Offline, on that sample, compressing each record on its own gives 1.76x.
Keeping a compressor across records, flushed after every record so the
record is complete when it is inserted, gives 2.93x across 8 streams.
The flush is not optional: WAL cannot leave a record's bytes inside a
compressor, and the compressed length has to be known before the space
is reserved.
The patch adds wal_compression_streams, a pool of streams a backend
leases while it compresses and inserts, so records enter a stream in the
order a reader will meet them. Currently a backend taking a lease over restarts
the stream, because the compressor state lives in the other backend and
shared memory cannot hold a libzstd object. I think I can fix it in future
iterations.
Once a record depends on the ones before it, some records have to stay
out of the scheme. The rule is not a resource-manager list but a
property: anything that someone reads by LSN without replaying what
precedes it has to stay readable on its own. Recovery finds the
checkpoint record from pg_control, and twophase.c reads a PREPARE record
from a stored LSN, so those are excluded, and XLOG_SWITCH with them.
This applies only with streams on; in 0002 every record is independent
and nothing is excluded.
What works: crash recovery, the data, amcheck, and reading a whole WAL
segment, because streams restart at segment boundaries.
What does not: a reader starting at an arbitrary LSN inside a segment
cannot decode, because it has no stream context for the records before
its start point. pg_waldump --start and pg_walinspect both fail that
way, and pg_rewind and logical decoding restart points would too. That
has to be solved before this is worth reviewing. I would welcome
opinions on the shape of the solution: my current thinking is that the
format has to let a reader find the nearest preceding stream restart.
It also costs throughput. On pgbench the stream lock, which is held
across compression and insertion, cost about 12% tps on my machine. I
have not measured that on a machine large enough for the WAL insert path
to be contended, and Andres has pointed out elsewhere that it already is
there.
---- Testing ----
Everything below is on a 16-core Linux box, built with AddressSanitizer
and --enable-cassert.
The workload walks the start offset of a compressed record across a
whole WAL page, emits messages sized on both sides of the threshold and
of the internal buffer bound, writes high-entropy payloads so the
compressor gives up, and takes four checkpoint-and-update rounds so
records carry full-page images. It then crashes the server and checks
that recovery reproduces two checksums, that pg_waldump and
pg_get_wal_records_info() decode the range, and that bt_index_check()
passes. wal_consistency_checking = all compares every replayed page
against the image in the record.
0001 and 0002 pass that with zero sanitizer reports, with compression
off, with zstd at the default and at threshold 32, with lz4, with pglz,
and with consistency checking on.
I also built all four combinations of --with-lz4 and --with-zstd with
-Werror, after Kirill's report above.
Two bugs in my own reader code turned up on the way, both now fixed in
0002. When a compressed record's header straddled a WAL page, the
reader reserved a decode slot sized for the compressed length and then
decoded the larger record into it. And the slot was not reset when the
read restarted, so a later pass could hand out a slot already in the
decode queue. Neither reproduces at the default threshold, which is why
they survived so long. I could not build a TAP test that fails before
the fix and passes after, so I have not added one.
---- Answers ----
On 12 Jan 2025, Kirill Reshke wrote:
> initdb fails when configured with --without-zstd
Thank you, and sorry it took this long. Builds without the compression
libraries are handled now: the whole-record machinery is compiled out
when neither lz4 nor zstd is present, and the reader still recognizes
such a record and reports it properly rather than mis-decoding it. I
built all four combinations with -Werror.
On 23 Jan 2025, Japin Li wrote:
> I see the patch compresses the WAL record according to the
> wal_compression, IIRC the wal_compression is only used for FPI, right?
> Maybe we should update the description of this parameter.
You are right, and I still owe this. postgresql.conf.sample is updated
in 0002, but the GUC description and the documentation are not. I will
do that before this soon.
> I see that the wal_compression_threshold defaults to 512. I wonder if
> you chose this value based on testing or randomly.
I answered this with a joke at the time, which it deserved less than a
real answer. I have measured it since: within per-record compression
the threshold barely matters, because small records do not compress on
their own. Compressing every record instead of only those over 512
bytes changed a 1.6GB sample by less than one percent. What the
threshold really controls is how much per-FPI compression gets
displaced, and lowering it can make WAL bigger, as in point 2 above.
For an outside data point, the usual recommendation for gzip over HTTP
is not to bother below roughly 860 bytes [1], on the same argument:
below that the header and the CPU cost outweigh what you save. That is
a different domain and WAL records are not HTTP responses, but the order
of magnitude agrees with what I measured. So 512 is a reasonable
default, for a better reason than the one I gave. If anything this
suggests it should be a little higher rather than lower, and I would not
object to 1kB if someone prefers a round number.
On 28 Jan 2025, Fujii Masao wrote:
> With the current approach, each backend needs to allocate memory twice
> the size of the total WAL record. [...] Perhaps we should consider
> setting a limit on the maximum memory each backend can use for WAL
> compression?
I tried that as a GUC and then removed it. The buffers are now a fixed
size, equal to the largest record that can be built, which is the same
amount the per-block arrays occupied at their maximum. A GUC did not
earn its place: the minimum useful value is already the value you want,
and it brought an assign-hook ordering bug with it, which Zsolt found.
The honest summary is point 1 above: it is 2x, not 1x, and I do not see
how to avoid the second buffer.
On 16 Jan 2026, Fujii Masao wrote:
> With the v5 patch, I see the following compiler warning:
> xloginsert.c:726:1: warning: unused function
> 'XLogGetRecordTotalLen'
Fixed since v6; it is guarded by WAL_DEBUG. Note my own tree builds
with -DWAL_DEBUG, so cfbot is what actually proves it.
> cfbot reported a regression test failure with v5.
Should be gone. make check and the recovery suite pass, and the stress
matrix above passes under a sanitizer build.
> When I ran pg_waldump on WAL generated with wal_compression=pglz and
> wal_compression_threshold=32, I got this error:
> pg_waldump: error: error in WAL record at 0/02183BE0: could not
> decompress record at 0/2183D10
> Isn't this a bug?
I could not reproduce it, and it is now moot for pglz specifically,
since pglz no longer takes part in whole-record compression. 0002 runs
pg_waldump over compressed WAL for every method as part of its test, and
the stress runs above do the same. If you still have the recipe I would
like to try it against v7.
> XLogEnsureCompressionBuffer() is now called every time
> XLogRegisterBuffer() [...] Why is that necessary?
It is not, and it is gone since v6. The buffers are allocated once,
outside any critical section.
> v5 removes the ability to compress only full-page images [...] Would
> it make more sense to keep the current behavior and add a new feature
> to compress entire WAL records whose size exceeds the specified
> threshold?
That was the right call and it is what 0002 does. Thank you.
On 9 Mar 2026, Zsolt Parragi wrote:
Thank you for reading it that closely. Every one of these was real.
> +static void
> +AllocCompressionBuffers(void)
> +{
> + uint32 new_size = wal_compression_buffer;
>
> This is called in the assign hook - and isn't that called before the
> global variable is updated?
Yes. guc.c calls the assign hook and then assigns the variable, so the
hook saw the old value, and raising the GUC did not grow the buffers.
XLogCompressRdt() would then flatten a larger record into a smaller
buffer, which is a heap overflow in a non-assert build. The GUC is gone
now, and the remaining hook reads newval.
> compressed_header[1] has an offset of 32, but compressed_data_size
> refers to the entire size, isn't there a possible buffer overrun here?
> Same with ZSTD.
Correct. The destination now gets the space that is really left, after
the header.
> This has 3 bytes of uninitialized padding, is that okay?
It was not. The header is zeroed before it is filled, so the bytes that
reach disk are deterministic. I also reordered the fields, and
SizeOfXLogCompressedRecord is now plain sizeof(), which keeps it in
agreement with the pointer the payload is written to.
> Doesn't this test needs a check to require a build with compression
> flags?
Fixed by using pglz for the suite-wide config, since it is always
available. 052 covers lz4 and zstd separately and skips what a build
lacks.
> But the default value is still off, isn't this example misleading?
That was a leftover from my testing. postgresql.conf.sample says off
again.
> + total_len_decomp = -1; /* XLogCompressionHeader spans pages */
> at multiple places, but this is an unsigned variable.
Replaced with an explicit boolean. While fixing it I found that the
branch it belonged to was wrong in a worse way: it reserved a decode
slot sized for the compressed length and then decoded the decompressed
record into it.
> Doesn't guc_params.dat support using macros instead of this hardcoded
> magic number?
It does, but the GUC is gone so the number with it.
On 10 Mar 2026, Japin Li wrote:
> Thanks for updating the patch. It seems a rebase is needed.
Done, on top of master as of this week. Sorry for the wait.
Thank you!
Best regards, Andrey Borodin.
[0] https://postgr.es/m/[email protected]
[1]
https://webmasters.stackexchange.com/questions/31750/what-is-recommended-minimum-object-size-for-gzip-performance-benefits
v7-0001-Reuse-a-zstd-compression-context-for-WAL-compress.patch
Description: Binary data
nocfbot-v7-0003-WIP-compress-WAL-records-against-earlier-records-.patch
Description: Binary data
v7-0002-Add-whole-record-WAL-compression-alongside-FPI-co.patch
Description: Binary data
