On Tue, Oct 06, 2026 at 06:09:38PM +0500, Andrey Borodin wrote: > The reader now owns a ZSTD_DCtx pointer under USE_ZSTD. Backend builds > register a reset callback in the reader's memory context, and > XLogReaderFree() unregisters it before freeing the context. I also > shortened the compression-side comment.
The declaration of zstd.h is actually quite invasive. Another possibility would be a forward declaration, like: typedef struct ZSTD_DCtx_s ZSTD_DCtx; That gives type enforcement, but perhaps you had the right idea in v9 where we used a void* pointer. I'd still keep that behind a USE_ZSTD, though, with a zstd-ish naming. I have also been benchmarking the replay path a bit with something like the following based on pgbench_accounts, that gives a fixed number of compressible pages, then run series of VACUUM FULL to generate WAL mostly made of full page records: CREATE TABLE t (aid int, bid int, abalance int, filler char(84)) WITH (autovacuum_enabled = off); INSERT INTO t SELECT i, i / 100000 + 1, 0, '' FROM generate_series(1, 610000) i; VACUUM t; CHECKPOINT; -- series 10k pages (see via pg_waldump -b). VACUUM FULL t; On replay (no fsync, -O2, no asserts, data on tmpfs), I am getting depending on two different hosts a gain of 1.5% and 3.7% in replay time (100 VACUUM FULLs, so 1M pages), which is not as much as you have reported, still that's above noise. For 50 lines of extra code, I'll take that. I would be interested in any scripts you may have, and attempt to reproduce the numbers you have posted. I'll also try some more runs on a third host (much better CPU and disk that I have around, locally, not in the cloud). > We could also avoid this cleanup bookkeeping with a static context, as > in 0002. For now we only decompress independent images in one call, with > no history to preserve between records. With xlogreader.c/h being a pluggable facility for frontend and backend, storing a static state is not really cool IMO. > My reasoning was that the writer can still store an uncompressed image, > whereas replay cannot restore a compressed image without the context. > I changed this as you suggested, but I still think falling back to an > uncompressed image is preferable. We are in a critical section. > > wal_compression can change during a session, so preparing the workspace > outside critical sections is not straightforward. This is also the > hardest and most contentious part of the whole-record and streaming > compression patches. I'm trying to reuse the FPI compression buffers, > but still need twice as much buffer space as before. I haven't found a > better approach. Your thoughts on the allocation strategy would be very > helpful, even without reviewing those patches. Hmm. OK, perhaps just storing it uncompressed is better if the context allocation fails, that's unlikely, still perhaps your alternative is just better than a plain crash. Please let me ponder on this part a bit more. I am attaching a slighly-edited v4 for the time being, which is fine to me code-wise. I am still planning to do a bit more benchmarking with replay times, but my first impression is that the numbers seem good. So, any thoughts or comments? -- Michael
From 8c774c8454c79a6f60cbfbcd53b08d2bcde7602e Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Wed, 7 Oct 2026 17:26:51 +0900 Subject: [PATCH v4] Reuse zstd decompression contexts when restoring WAL images Restoring each zstd-compressed full-page image currently creates and destroys a decompression context. Avoid that overhead by keeping one context per WAL reader, allocated on first use. Free the context in XLogReaderFree(). In backend builds, also register a reset callback with the reader's memory context so that errors do not leak the allocation made by zstd. Author: Andrey Borodin <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Discussion: https://www.postgresql.org/message-id/[email protected] --- src/include/access/xlogreader.h | 9 +++++ src/backend/access/transam/xlogreader.c | 51 +++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index 4a9a687e8796..64a9492f6ed5 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -239,6 +239,15 @@ struct XLogReaderState * ---------------------------------------- */ +#ifdef USE_ZSTD + /* Decompression context reused for zstd-compressed full-page images. */ + void *zstd_dctx; +#ifndef FRONTEND + /* Reset callback for zstd_dctx */ + MemoryContextCallback zstd_dctx_cb; +#endif +#endif + /* * Buffer for decoded records. This is a circular buffer, though * individual records can't be split in the middle, so some space is often diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 7db7c273b0c6..a8407be09fc3 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -36,6 +36,7 @@ #ifndef FRONTEND #include "pgstat.h" #include "storage/bufmgr.h" +#include "utils/memutils.h" #include "utils/wait_event.h" #else #include "common/logging.h" @@ -55,6 +56,9 @@ static bool ValidXLogRecord(XLogReaderState *state, XLogRecord *record, static void ResetDecoder(XLogReaderState *state); static void WALOpenSegmentInit(WALOpenSegment *seg, WALSegmentContext *segcxt, int segsize, const char *waldir); +#if defined(USE_ZSTD) && !defined(FRONTEND) +static void XLogReaderFreeZstdContext(void *arg); +#endif /* size of the buffer allocated for error message. */ #define MAX_ERRORMSG_LEN 1000 @@ -171,10 +175,29 @@ XLogReaderFree(XLogReaderState *state) pfree(state->errormsg_buf); if (state->readRecordBuf) pfree(state->readRecordBuf); +#ifdef USE_ZSTD + if (state->zstd_dctx != NULL) + { +#ifndef FRONTEND + MemoryContextUnregisterResetCallback(GetMemoryChunkContext(state), + &state->zstd_dctx_cb); +#endif + ZSTD_freeDCtx(state->zstd_dctx); + } +#endif pfree(state->readBuf); pfree(state); } +#if defined(USE_ZSTD) && !defined(FRONTEND) +/* Release the zstd decomoression context. */ +static void +XLogReaderFreeZstdContext(void *arg) +{ + ZSTD_freeDCtx(arg); +} +#endif + /* * Allocate readRecordBuf to fit a record of at least the given length. * @@ -2177,9 +2200,31 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page) else if ((bkpb->bimg_info & BKPIMAGE_COMPRESS_ZSTD) != 0) { #ifdef USE_ZSTD - size_t decomp_result = ZSTD_decompress(tmp.data, - BLCKSZ - bkpb->hole_length, - ptr, bkpb->bimg_len); + size_t decomp_result; + + if (record->zstd_dctx == NULL) + { + record->zstd_dctx = ZSTD_createDCtx(); + if (record->zstd_dctx == NULL) + { + report_invalid_record(record, "out of memory while restoring image at %X/%08X, block %d", + LSN_FORMAT_ARGS(record->ReadRecPtr), + block_id); + return false; + } + +#ifndef FRONTEND + /* The reader may outlive the current memory context. */ + record->zstd_dctx_cb.func = XLogReaderFreeZstdContext; + record->zstd_dctx_cb.arg = record->zstd_dctx; + MemoryContextRegisterResetCallback(GetMemoryChunkContext(record), + &record->zstd_dctx_cb); +#endif + } + + decomp_result = ZSTD_decompressDCtx(record->zstd_dctx, tmp.data, + BLCKSZ - bkpb->hole_length, + ptr, bkpb->bimg_len); if (ZSTD_isError(decomp_result)) decomp_success = false; -- 2.55.0
signature.asc
Description: PGP signature
