Hi hackers, While reviewing the backup compression backends, I noticed that the gzip sink (basebackup_gzip.c) never calls deflateEnd(), unlike the lz4 and zstd sinks which properly free their compression contexts.
Specifically:
- lz4 calls LZ4F_freeCompressionContext() in both end_archive and a
dedicated bbsink_lz4_cleanup() callback.
- zstd calls ZSTD_freeCCtx() in both end_archive and a dedicated
bbsink_zstd_cleanup() callback.
- gzip calls deflateInit2() in begin_archive but never calls
deflateEnd(). Its cleanup callback is bbsink_forward_cleanup,
which just forwards to the next sink without releasing the zlib
context.
This means each archive (one per tablespace) leaks ~256KB of zlib
internal state until the backup's memory context is destroyed. On the
ERROR path, the zlib context is not released at all since there is no
gzip-specific cleanup callback.
The attached patch adds deflateEnd() at the end of
bbsink_gzip_end_archive() for the normal path, and introduces a new
bbsink_gzip_cleanup() for the error path. The new cleanup callback
follows the exact same call chain as bbsink_lz4_cleanup and
bbsink_zstd_cleanup:
PG_FINALLY (basebackup.c:1063)
-> bbsink_cleanup(sink) (basebackup.c:1065)
-> sink->bbs_ops->cleanup(sink) (basebackup_sink.h:269)
-> bbsink_gzip_cleanup() -- now calls deflateEnd()
Previously the gzip slot in this chain pointed to
bbsink_forward_cleanup, which just forwarded to the next sink
without doing any gzip-specific resource release.
Since z_stream is an embedded struct (not a pointer like LZ4F_cctx or
ZSTD_CCtx), a bool flag "zstream_initialized" is used to track whether
deflateEnd() needs to be called.
Tested with pg_basebackup using gzip compression at levels 1, 5
(default), and 9, including server-side compression. All produced
valid .tar.gz files that pass gzip -t integrity checks and restore
correctly.
Regards,
Jianghua Yang
0001-basebackup-add-missing-deflateEnd-calls-in-gzip-comp.patch
Description: Binary data
