Hi, On Tue, Jul 14, 2026 at 8:21 AM Robert Haas <[email protected]> wrote: > > On Wed, Mar 25, 2026 at 5:32 PM SATYANARAYANA NARLAPURAM > <[email protected]> wrote: > > Both pg_prewarm() and the autoprewarm background worker hold > > AccessShareLock on the target relation for the entire duration of > > prewarming. On large tables this can take a long time, which means > > that any DDL that needs a stronger lock (TRUNCATE, DROP TABLE, ALTER TABLE, > > etc.) is blocked for the full duration.
Thanks Satya for the off-list discussion, and thanks Robert for the review. > This patch goes to quite a bit of trouble to restart prewarming of a > relation after releasing and reacquiring the lock. I feel like that's > adding a lot of complexity of questionable value. I think I'd be > inclined not to change the foreground path at all, just like a > foreground VACUUM doesn't do anything special to deprioritize itself, > and make the autoprewarm give up on the relation entirely if someone > else wants the lock, just like what autovacuum does. Agreed on keeping the behavior in sync with vacuum. Rather than the autovacuum's cancellation via PROC_IS_AUTOVACUUM, I used the vacuum's truncation approach of calling LockHasWaitersRelation() to detect waiters, checking every 32 blocks and at most every 20ms. Please let me know if those intervals need to be larger, or if there's a better idea here. With this approach, autoprewarm may leave already-loaded blocks of the relation in the buffer pool after giving it up. We could evict them, even after releasing the lock so the waiter isn't delayed, but that feels like overkill IMO, and vacuum leaves blocks behind in the same way anyway. > If we do it like > this, I think we need a really good argument for handling this case > differently from autovacuum. If somebody takes AccessExclusiveLock on > a relation, there's a good chance that the block numbers we have are > not even relevant any more afterwards. IMHO this behavior is simple to reason about, and it avoids the problems that a concurrent rewrite can cause. > On a purely mechanical note, this patch results in a block of code in > autoprewarm_database_main() that currently looks very simple looking > extremely complicated instead. The purpose of that code is not so > obvious any more, and there's a lot of extra indentation that impacts > readability. If you want to pursue this, I suggest thinking about how > you could introduce subroutines or otherwise refactor so that a future > human reader will be able to understand this nearly as easily as they > can understand the current code. I moved that logic into a separate function to keep autoprewarm_database_main() readable. Please find the attached v2 patches. 0002 is a TAP test that I don't intend to get this committed, as it relies on a very large table that doesn't fit well with the overall test timing. -- Bharath Rupireddy Amazon Web Services: https://aws.amazon.com
From 999e44caaef12e530e42463e0f764bd7391ca510 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Sun, 2 Aug 2026 03:56:49 +0000 Subject: [PATCH v2 1/2] Make autoprewarm yield to conflicting lock requests. The autoprewarm worker holds AccessShareLock on a relation while reading all of its previously-dumped blocks back into shared buffers. On a large relation this takes a long time, and any concurrent operation needing a conflicting lock, such as TRUNCATE or DROP, is blocked for the whole duration. Have the worker periodically check whether a conflicting lock request is waiting and, if so, give up the relation: release the lock and move on to the next one. Prewarming is best-effort, and once a conflicting strong lock is taken the recorded block numbers may no longer be useful, so there is little value in reacquiring the lock to finish. This mirrors how VACUUM behaves during heap truncation. The check is throttled to keep its cost negligible when nobody is waiting. The foreground pg_prewarm() function is left unchanged, on the same reasoning that a foreground VACUUM does not deprioritize itself: its caller asked for a bounded amount of work and is waiting on it. Author: Bharath Rupireddy <[email protected]> Co-authored-by: Satyanarayana Narlapuram <[email protected]> Reviewed-by: Robert Haas <[email protected]> Discussion: https://www.postgresql.org/message-id/flat/CAHg%2BQDfdoR%3D7iqEAvLW9qtzV0Sx1wp2FuALeamqcCdiVEmMF-Q%40mail.gmail.com --- contrib/pg_prewarm/autoprewarm.c | 109 +++++++++++++++++++++++++------ 1 file changed, 89 insertions(+), 20 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index deb4c2671b5..0e909e34be2 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -31,6 +31,7 @@ #include "access/relation.h" #include "access/xact.h" #include "pgstat.h" +#include "portability/instr_time.h" #include "postmaster/bgworker.h" #include "postmaster/interrupt.h" #include "storage/buf_internals.h" @@ -39,6 +40,7 @@ #include "storage/fd.h" #include "storage/ipc.h" #include "storage/latch.h" +#include "storage/lmgr.h" #include "storage/lwlock.h" #include "storage/procsignal.h" #include "storage/read_stream.h" @@ -52,6 +54,14 @@ #define AUTOPREWARM_FILE "autoprewarm.blocks" +/* + * How often the prewarm loop checks for a conflicting lock request: probe the + * shared lock table at most once per interval (in ms), and only consult the + * clock once every so many blocks. + */ +#define PREWARM_LOCK_CHECK_INTERVAL 20 /* ms */ +#define PREWARM_LOCK_CHECK_BLOCKS 32 + /* Metadata for each block we dump. */ typedef struct BlockInfoRecord { @@ -493,6 +503,73 @@ apw_read_stream_next_block(ReadStream *stream, return InvalidBlockNumber; } +/* + * Prewarm the blocks of one fork by draining the read stream, and return true + * if a conflicting lock request showed up while doing so. On such a waiter we + * stop early rather than reacquiring the lock and resuming; prewarming is + * best-effort, and the caller gives up the relation to let the waiter proceed. + * The read stream is always shut down before returning. + */ +static bool +apw_prewarm_blocks(Relation rel, struct AutoPrewarmReadStreamData *p) +{ + ReadStream *stream; + Buffer buf; + instr_time starttime; + int blocks_since_check = 0; + bool waiter_detected = false; + + stream = read_stream_begin_relation(READ_STREAM_MAINTENANCE | + READ_STREAM_DEFAULT | + READ_STREAM_USE_BATCHING, + NULL, + rel, + p->forknum, + apw_read_stream_next_block, + p, + 0); + + INSTR_TIME_SET_CURRENT(starttime); + + while ((buf = read_stream_next_buffer(stream, NULL)) != InvalidBuffer) + { + apw_state->prewarmed_blocks++; + ReleaseBuffer(buf); + + /* + * Check for a conflicting lock waiter, but keep the clock reads and + * lock table probes rare: only look at the clock every + * PREWARM_LOCK_CHECK_BLOCKS blocks, and only probe once + * PREWARM_LOCK_CHECK_INTERVAL has elapsed since the last probe. + */ + if (++blocks_since_check >= PREWARM_LOCK_CHECK_BLOCKS) + { + instr_time currenttime; + instr_time elapsed; + + blocks_since_check = 0; + + INSTR_TIME_SET_CURRENT(currenttime); + elapsed = currenttime; + INSTR_TIME_SUBTRACT(elapsed, starttime); + if ((INSTR_TIME_GET_MICROSEC(elapsed) / 1000) + >= PREWARM_LOCK_CHECK_INTERVAL) + { + if (LockHasWaitersRelation(rel, AccessShareLock)) + { + waiter_detected = true; + break; + } + starttime = currenttime; + } + } + } + + read_stream_end(stream); + + return waiter_detected; +} + /* * Prewarm all blocks for one database (and possibly also global objects, if * those got grouped with this database). @@ -577,8 +654,6 @@ autoprewarm_database_main(Datum main_arg) ForkNumber forknum; BlockNumber nblocks; struct AutoPrewarmReadStreamData p; - ReadStream *stream; - Buffer buf; blk = block_info[i]; @@ -627,29 +702,23 @@ autoprewarm_database_main(Datum main_arg) .nblocks = nblocks, }; - stream = read_stream_begin_relation(READ_STREAM_MAINTENANCE | - READ_STREAM_DEFAULT | - READ_STREAM_USE_BATCHING, - NULL, - rel, - p.forknum, - apw_read_stream_next_block, - &p, - 0); - /* - * Loop until we've prewarmed all the blocks from this fork. The - * read stream callback will check that we still have free buffers - * before requesting each block from the read stream API. + * On a conflicting lock waiter, give up the whole relation: skip + * its remaining blocks and break out so we close it (releasing + * the lock) and move on to the next one. */ - while ((buf = read_stream_next_buffer(stream, NULL)) != InvalidBuffer) + if (apw_prewarm_blocks(rel, &p)) { - apw_state->prewarmed_blocks++; - ReleaseBuffer(buf); + for (i = p.pos; i < apw_state->prewarm_stop_idx; i++) + { + blk = block_info[i]; + if (blk.tablespace != tablespace || + blk.filenumber != filenumber) + break; + } + break; } - read_stream_end(stream); - /* * Advance i past all the blocks just prewarmed. Note that the * callback might have advanced the index beyond the last valid -- 2.47.3
From e007ebf9e56f3bb324dd46469587dc74335fe093 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Sun, 2 Aug 2026 03:57:56 +0000 Subject: [PATCH v2 2/2] Add test for autoprewarm yielding to conflicting lock requests. Add an injection point in the autoprewarm worker's block-read loop and a TAP test that uses it. The test pauses the worker mid-prewarm, starts a TRUNCATE that blocks on the conflicting lock, then resumes the worker and checks that it releases its lock, lets the TRUNCATE finish, and gives up the relation (reporting fewer prewarmed blocks than it dumped). The worker only loads the dump at startup and injection points do not survive a restart, so the wait can only be armed after the restart. A large table and buffer pool keep the scan running long enough to attach the injection point and still catch a later check. That fixed size makes the test too heavy for the buildfarm; it is meant to be run locally. Author: Bharath Rupireddy <[email protected]> Discussion: https://www.postgresql.org/message-id/flat/CAHg%2BQDfdoR%3D7iqEAvLW9qtzV0Sx1wp2FuALeamqcCdiVEmMF-Q%40mail.gmail.com --- contrib/pg_prewarm/Makefile | 3 + contrib/pg_prewarm/autoprewarm.c | 3 + contrib/pg_prewarm/meson.build | 4 + .../t/002_autoprewarm_lock_yield.pl | 101 ++++++++++++++++++ 4 files changed, 111 insertions(+) create mode 100644 contrib/pg_prewarm/t/002_autoprewarm_lock_yield.pl diff --git a/contrib/pg_prewarm/Makefile b/contrib/pg_prewarm/Makefile index 617ac8e09b2..53bce44971a 100644 --- a/contrib/pg_prewarm/Makefile +++ b/contrib/pg_prewarm/Makefile @@ -12,6 +12,9 @@ PGFILEDESC = "pg_prewarm - preload relation data into system buffer cache" REGRESS = pg_prewarm +EXTRA_INSTALL = src/test/modules/injection_points +export enable_injection_points + TAP_TESTS = 1 ifdef USE_PGXS diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index 0e909e34be2..4a9f182a6e7 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -47,6 +47,7 @@ #include "storage/smgr.h" #include "tcop/tcopprot.h" #include "utils/guc.h" +#include "utils/injection_point.h" #include "utils/rel.h" #include "utils/relfilenumbermap.h" #include "utils/timestamp.h" @@ -549,6 +550,8 @@ apw_prewarm_blocks(Relation rel, struct AutoPrewarmReadStreamData *p) blocks_since_check = 0; + INJECTION_POINT("autoprewarm-before-lock-check", NULL); + INSTR_TIME_SET_CURRENT(currenttime); elapsed = currenttime; INSTR_TIME_SUBTRACT(elapsed, starttime); diff --git a/contrib/pg_prewarm/meson.build b/contrib/pg_prewarm/meson.build index e70546a451b..e43b9b2e1b8 100644 --- a/contrib/pg_prewarm/meson.build +++ b/contrib/pg_prewarm/meson.build @@ -35,8 +35,12 @@ tests += { ], }, 'tap': { + 'env': { + 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no', + }, 'tests': [ 't/001_basic.pl', + 't/002_autoprewarm_lock_yield.pl', ], }, } diff --git a/contrib/pg_prewarm/t/002_autoprewarm_lock_yield.pl b/contrib/pg_prewarm/t/002_autoprewarm_lock_yield.pl new file mode 100644 index 00000000000..f66de6daf17 --- /dev/null +++ b/contrib/pg_prewarm/t/002_autoprewarm_lock_yield.pl @@ -0,0 +1,101 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test that the autoprewarm worker gives up a relation when a conflicting +# lock request is waiting, letting the DDL proceed. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + +my $node = PostgreSQL::Test::Cluster->new('main'); +$node->init; + +# The worker only loads the dump at startup and injection points do not +# survive a restart, so the wait can only be armed after the restart below. +# A large table and buffer pool keep the scan running long enough to attach +# the injection point and still catch a later check. That size makes this a +# heavy, manual/local test rather than one for the buildfarm. +$node->append_conf( + 'postgresql.conf', qq{ +shared_preload_libraries = 'pg_prewarm,injection_points' +pg_prewarm.autoprewarm = true +pg_prewarm.autoprewarm_interval = 0 +autovacuum = off +shared_buffers = '2GB' +}); +$node->start; + +# The injection_points extension may not be installed under installcheck. +if (!$node->check_extension('injection_points')) +{ + plan skip_all => 'Extension injection_points not installed'; +} + +$node->safe_psql('postgres', q( + CREATE EXTENSION pg_prewarm; + CREATE EXTENSION injection_points; +)); + +$node->safe_psql('postgres', q( + CREATE TABLE t (id int, pad text); + INSERT INTO t SELECT g, repeat('x', 500) + FROM generate_series(1, 1000000) g; +)); + +my $nblocks = $node->safe_psql('postgres', + "SELECT pg_relation_size('t') / current_setting('block_size')::int"); +ok($nblocks > 32, "table t has more than 32 blocks ($nblocks)"); + +# Warm t and record its blocks so the worker reloads them after a restart. +$node->safe_psql('postgres', "SELECT pg_prewarm('t', 'buffer')"); +my $dumped = $node->safe_psql('postgres', "SELECT autoprewarm_dump_now()"); +ok($dumped > 32, "autoprewarm dumped more than 32 blocks ($dumped)"); + +$node->restart; + +# Pause the worker mid-prewarm. +$node->safe_psql('postgres', + "SELECT injection_points_attach('autoprewarm-before-lock-check', 'wait')"); +$node->wait_for_event('autoprewarm worker', 'autoprewarm-before-lock-check'); + +# TRUNCATE now blocks on the AccessExclusiveLock the worker conflicts with. +my $truncate = $node->background_psql('postgres'); +$truncate->query_until(qr/starting_truncate/, q( + \echo starting_truncate + TRUNCATE t; +)); +$node->poll_query_until('postgres', q( + SELECT count(*) > 0 FROM pg_stat_activity + WHERE query LIKE '%TRUNCATE t%' AND wait_event_type = 'Lock'; +)) or die "timed out waiting for TRUNCATE to block on the lock"; + +# Resume the worker; it should see the waiter and release its lock. +my $log_offset = -s $node->logfile; +$node->safe_psql('postgres', + "SELECT injection_points_detach('autoprewarm-before-lock-check')"); +$node->safe_psql('postgres', + "SELECT injection_points_wakeup('autoprewarm-before-lock-check')"); + +$truncate->quit; +pass('TRUNCATE completed while autoprewarm worker was prewarming'); + +# Having given up t, the worker warmed fewer blocks than it dumped. +$node->wait_for_log( + qr/autoprewarm successfully prewarmed \d+ of \d+ previously-loaded blocks/, + $log_offset); +my $summary = slurp_file($node->logfile, $log_offset); +my ($prewarmed, $total) = $summary =~ + /successfully prewarmed (\d+) of (\d+) previously-loaded blocks/; +cmp_ok($prewarmed, '<', $total, + "worker gave up early: prewarmed $prewarmed of $total blocks"); + +$node->stop; +done_testing(); -- 2.47.3
