From cab8de33d49b66eb264a6a8a5b1e85e73356dc39 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 3 Aug 2026 17:06:30 -0400 Subject: [PATCH] Restore vacuum failsafe abandonment of buffer access strategy VACUUM's wraparound failsafe mode exists to reclaim transaction IDs as quickly as possible. 4830f1024325 made the failsafe stop using the BAS_VACUUM buffer access strategy so that the rest of the vacuum could make use of all of shared buffers rather than being confined to the small strategy ring. However, when 9256822608f3 made vacuum's first heap pass use the read stream, this was accidentally disabled. The read stream keeps its own references to the buffer access strategy, so clearing vacrel->bstrategy in lazy_check_wraparound_failsafe() no longer had any effect on the reads issued by the first pass. Fix this by adding clearing the BufferAccessStrategy reference actually being used by the ongoing scan -- those in the ReadBuffersOperations structs themselves. Two things we accept rather than fix, as neither is worth the added complexity given how rarely failsafe mode is reached: - A small amount of read time for IOs that were already in progress when the strategy was cleared may be attributed to IOCONTEXT_NORMAL instead of IOCONTEXT_VACUUM. WaitReadBuffers() derives the IOContext from the (now cleared) strategy, so the wait time of these in-flight IOs is misattributed. This is bounded by the stream's look-ahead window and happens at most once per vacuum, when the strategy is first cleared. - The stream's buffer pin limit stays lower than it would have been had no strategy been used at all. max_pinned_buffers is capped by the strategy's pin limit when the stream is created and is not recomputed when the strategy is cleared. Raising it would mean building a new, larger ring, which would require first waiting for all in-progress IOs to complete. That didn't seem worth it. Reported-by: Jingtang Zhang Discussion: https://postgr.es/m/CAPsk3_APRYVLhAJ5TMwdmpSx8W_%3DPHMm%3DPmKAvnC3gBrfNommQ%40mail.gmail.com Backpatch-to: 18 --- src/backend/access/heap/vacuumlazy.c | 24 +++++++++++++++++++++--- src/backend/storage/aio/read_stream.c | 21 +++++++++++++++++++++ src/include/storage/read_stream.h | 1 + 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 39395aed0d5..2fb16710435 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1385,6 +1385,19 @@ lazy_scan_heap(LVRelState *vacrel) PROGRESS_VACUUM_PHASE_SCAN_HEAP); } + /* + * If the wraparound failsafe has engaged -- either via the check + * above or during index vacuuming invoked from this loop -- stop + * using the buffer access strategy so that the rest of the vacuum may + * use all of shared buffers. We have to clear the reference to the + * strategy actually being used by the read stream. This couldn't be + * done in lazy_check_wraparound_failsafe() because it may be called + * from any phase of vacuum, including those when this stream is + * inactive. + */ + if (unlikely(VacuumFailsafeActive)) + read_stream_clear_strategy(stream); + buf = read_stream_next_buffer(stream, &per_buffer_data); /* The relation is exhausted. */ @@ -2905,9 +2918,14 @@ lazy_check_wraparound_failsafe(LVRelState *vacrel) VacuumFailsafeActive = true; /* - * Abandon use of a buffer access strategy to allow use of all of - * shared buffers. We assume the caller who allocated the memory for - * the BufferAccessStrategy will free it. + * We abandon use of the strategy in failsafe mode to allow use of all + * of shared buffers. vacrel->bstrategy is not the source of truth for + * an ongoing heap scan, but clear it just for tidiness. Any ongoing + * phase I heap scan has its own references to the strategy and clears + * it separately (see lazy_scan_heap()). And none of the other vacuum + * phases will read from vacrel->bstrategy once failsafe mode is + * engaged. We assume the caller who allocated the memory for the + * BufferAccessStrategy will free it. */ vacrel->bstrategy = NULL; diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index a318539e56c..e7dbbe03326 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -1407,6 +1407,27 @@ read_stream_resume(ReadStream *stream) stream->combine_distance = stream->resume_combine_distance; } +/* + * Stop using a buffer access strategy for reads from this stream. + * + * This clears the strategy for all of the stream's ReadBuffersOperations, + * including those with in-progress IOs. The completion of an IO whose + * strategy was cleared while it was in flight may have a small amount of its + * read time attributed to IOCONTEXT_NORMAL instead of the strategy's + * IOContext, because WaitReadBuffers() derives the IOContext from the (now + * cleared) strategy. This is bounded by the stream's look-ahead window and + * happens at most once, when the strategy is first cleared, so it is not worth + * the complexity of preserving the original IOContext for those IOs. + * + * Note that the caller is responsible for freeing the strategy's memory. + */ +void +read_stream_clear_strategy(ReadStream *stream) +{ + for (int i = 0; i < stream->max_ios; ++i) + stream->ios[i].op.strategy = NULL; +} + /* * Reset a read stream by releasing any queued up buffers, allowing the stream * to be used again for different blocks. This can be used to clear an diff --git a/src/include/storage/read_stream.h b/src/include/storage/read_stream.h index 48995c6d534..e2dcf1be50a 100644 --- a/src/include/storage/read_stream.h +++ b/src/include/storage/read_stream.h @@ -102,6 +102,7 @@ extern ReadStream *read_stream_begin_smgr_relation(int flags, size_t per_buffer_data_size); extern BlockNumber read_stream_pause(ReadStream *stream); extern void read_stream_resume(ReadStream *stream); +extern void read_stream_clear_strategy(ReadStream *stream); extern void read_stream_reset(ReadStream *stream); extern void read_stream_end(ReadStream *stream); extern void read_stream_enable_stats(ReadStream *stream, struct IOStats *stats); -- 2.47.3