Make pg_prewarm, autoprewarm yield for waiting DDL

Hi Satya,

On Thu, Mar 26, 2026 at 3:02 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> 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.

This indeed seems like a valid concern.

VACUUM already solves this same problem during heap truncation: it periodically calls LockHasWaitersRelation() and backs off when a conflicting waiter is detected (see lazy_truncate_heap()).

Yes, in the current design, waiter-aware backoff logic is present in
some code paths (like VACUUM) that acquire the strongest lock
(AccessExclusiveLock), but is largely absent from paths that hold
weaker locks.

While AccessShareLock conflicts with only one lock mode
(AccessExclusiveLock), a long-held AccessShareLock, as in the
pg_prewarm case you mentioned, can still cause meaningful delays for
DDL or maintenance operations that require AccessExclusiveLock. So
despite its narrow conflict set, the practical impact can be quite
significant in some cases. On that basis, extending waiter-aware
behavior to places like pg_prewarm (or similar long-running lock
holders) seems reasonable, though it would be worth seeing how others
think about it.

--
With Regards,
Ashutosh Sharma.

Hi,

On Wed, Mar 25, 2026 at 2:32 PM SATYANARAYANA NARLAPURAM <
satyanarlapuram@gmail.com> 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.

VACUUM already solves this same problem during heap truncation: it

periodically calls LockHasWaitersRelation() and backs off when a
conflicting waiter is detected (see lazy_truncate_heap()).

The attached patch applies the same pattern to pg_prewarm and

autoprewarm. Every 1024 blocks, each code path checks for a waiter and if
found then the lock is released so that the DDL can proceed. Patch handles
the relation truncation, drop cases by emitting an error message. If the
relation was only partially truncated, the endpoint is adjusted downward
and prewarming continues.

When no DDL is waiting, the only overhead is one lock-table probe per

1024 blocks.

A few comments:

1/
+/*
+ * Block interval for checking conflicting lock waiters during prewarming.
+ */
+#define PREWARM_WAITER_CHECK_INTERVAL 1024

Is this a random number, or was it chosen based on something like all 1024
pages being read from storage on a buffer pool miss and the time that
takes? I mainly want to check that this interval doesn't give too wide a
window, making a DDL wait longer than necessary. It's worth quickly
checking the worst case where all 1024 pages are read from storage.

Or, why not do something like what vacuum does - check every 32 blocks
whether the elapsed time of 20ms since the last check exceeds a threshold,
keeping the number of system calls and lock table lookups to a minimum?
2/
+ /*
+ * Recalculate fork size; skip remainder if
+ * truncated.
+ */
+ if (!smgrexists(RelationGetSmgr(rel), forknum))
+ {

I want to make sure I understand the table rewrite handling. If a
concurrent rewrite (ALTER TABLE, REINDEX, VACUUM FULL) happens while we're
prewarming, we release the lock and reopen the relation by OID. The reopen
succeeds because the OID is still valid, and the fork existence check
passes because relcache now points at the new relfilenode. But the block
list we're streaming was collected from the old relfilenode - so we end up
reading the new file at the old block numbers, and never actually prewarm
the right buffers. Am I reading that correctly?

Also, the reopen-and-restart path is fairly complex to reason about. Since
prewarming is best-effort anyway, would it be simpler to just release the
lock when we see a waiter, drop the blocks already collected for that
relation from the prewarm list,
and move on to the next relation? A relation that doesn't fully get
prewarmed isn't a correctness problem. Was there a specific reason for the
more involved restart approach?

The patch includes a TAP test (t/002_lock_yield.pl) that exercises the

TRUNCATE and DROP TABLE scenarios using injection points.

Can we separate the injection point and test into a 0002 patch?

While developing this patch I discovered that LockHasWaiters() crashes

with a segfault when the lock in question was acquired via the fast-path
optimization, details in [1].

I agree to fix that separately. I previously reviewed and tested the patch
there and it LGTM.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

On Wed, Mar 25, 2026 at 5:32 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> 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.

VACUUM already solves this same problem during heap truncation: it periodically calls LockHasWaitersRelation() and backs off when a conflicting waiter is detected (see lazy_truncate_heap()).

The attached patch applies the same pattern to pg_prewarm and autoprewarm. Every 1024 blocks, each code path checks for a waiter and if found then the lock is released so that the DDL can proceed. Patch handles the relation truncation, drop cases by emitting an error message. If the relation was only partially truncated, the endpoint is adjusted downward and prewarming continues.
When no DDL is waiting, the only overhead is one lock-table probe per 1024 blocks.

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. 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.

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.

--
Robert Haas
EDB: http://www.enterprisedb.com

Hi,

On Tue, Jul 14, 2026 at 8:21 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Mar 25, 2026 at 5:32 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> 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

Hi Bharath, Satyanarayana,

I picked this up from the August CF and reproduced the CFBot "Test
world" failure locally on postgres/master at 3d00537feb5 with 0001
and 0002 applied. I am writing this as a review rather than a v3,
since you said 0002 is not meant for commit.

Reproducer
==========

Fresh worktree from master, cherry-picked a867de4b350 (code) and
a08931a23ba (test) from cfbot/cf/7098. Built with
-Dinjection_points=true, then:

enable_injection_points=yes meson test -C build --suite pg_prewarm

001_basic passes. 002_autoprewarm_lock_yield times out with
"timed out waiting for TRUNCATE to block on the lock".

What is happening
=================

The wait uses:

$node->wait_for_event('autoprewarm worker',
'autoprewarm-before-lock-check');

but apw_prewarm_blocks() reaches that injection point every 32
blocks for every relation in the dump, not just t. After the
1M-row INSERT and CREATE EXTENSIONs, pg_attribute already has
enough blocks to hit the point before the worker gets to t.
wait_for_event() matches that hit, TRUNCATE runs unopposed and
the poll for wait_event_type = 'Lock' times out.

A pg_stat_activity + pg_locks snapshot at the failure point
confirms it:

autoprewarm worker | InjectionPoint | autoprewarm-before-lock-check
client backend | idle | ClientRead | TRUNCATE t;
pg_locks: worker holds AccessShareLock on pg_attribute, not t.

Suggested test fix
==================

The wait needs two things: the worker is paused at the injection
point, and it currently holds AccessShareLock on t. If neither is
true yet, wake the point so the worker advances to the next
check. Something like:

$node->poll_query_until('postgres', q(
SELECT CASE
WHEN EXISTS (
SELECT 1
FROM pg_locks l JOIN pg_stat_activity a USING (pid)
WHERE a.backend_type = 'autoprewarm worker'
AND a.wait_event = 'autoprewarm-before-lock-check'
AND l.relation = 't'::regclass
AND l.mode = 'AccessShareLock'
AND l.granted)
THEN true
WHEN EXISTS (
SELECT 1 FROM pg_stat_activity
WHERE backend_type = 'autoprewarm worker'
AND wait_event = 'autoprewarm-before-lock-check')
THEN injection_points_wakeup('autoprewarm-before-lock-check')
IS NOT NULL
ELSE false
END));

A pure poll on the lock is not enough on its own, because the
worker stays frozen at the first wrong-relation hit until it is
woken.

0002 is manual-only per your note, but CFBot still runs it and
that is what turns the entry red, so this seems worth fixing.

Prerequisite: LockHasWaiters() fast-path crash (CF #6732)
=========================================================

With the synchronization above in place, the worker reaches its
real waiter check and terminates with:

background worker "autoprewarm worker" (PID ...) was
terminated by signal 11: Segmentation fault

apw_prewarm_blocks() calls LockHasWaitersRelation(), which is a
thin wrapper around LockHasWaiters(). Its fast-path crash is the
same bug you already have open in CF #6732. In other words,
0001's runtime behavior depends on that fix; it should probably
be called out as a prerequisite in the commit message, and I
think it is worth merging the two efforts (or at least ordering
them) rather than committing 0001 first.

I did not manage to get a green 002 run on the current branch
without CF #6732 applied. Happy to rerun once that lands.

Thanks,
Palak

On Tue, 4 Aug 2026 at 05:11, Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

Hi,

On Mon, Aug 17, 2026 at 6:44 AM Palak Chaturvedi
<chaturvedipalak1911@gmail.com> wrote:

I picked this up from the August CF and reproduced the CFBot "Test
world" failure locally on postgres/master at 3d00537feb5 with 0001
and 0002 applied. I am writing this as a review rather than a v3,
since you said 0002 is not meant for commit.

Thanks for taking a look at it.

The wait needs two things: the worker is paused at the injection
point, and it currently holds AccessShareLock on t. If neither is
true yet, wake the point so the worker advances to the next
check. Something like:

I think passing the relation name as the injection point argument
(commit 0fd73cdffc1) makes the injection point fire only while the
worker is scanning the required table, so it can no longer pause on an
earlier relation and the race goes away. Does that work for you?

I moved the LockHasWaiters() fix needed from
https://commitfest.postgresql.org/patch/6732/ here and made it the
0001 patch.

I couldn't find a better way to make the test deterministic without a
large table, because the injection point can only be attached after
the restart when the worker is already scanning, so a long scan is
what gives the test enough time to attach before the worker finishes.
I'm open to thoughts here.

Please find the attached v3 patches.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

Hi Bharath,

On Wed, 2 Sept 2026 at 16:23, Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

Hi,

On Mon, Aug 17, 2026 at 6:44 AM Palak Chaturvedi
<chaturvedipalak1911@gmail.com> wrote:

I picked this up from the August CF and reproduced the CFBot "Test
world" failure locally on postgres/master at 3d00537feb5 with 0001
and 0002 applied. I am writing this as a review rather than a v3,
since you said 0002 is not meant for commit.

Thanks for taking a look at it.

The wait needs two things: the worker is paused at the injection
point, and it currently holds AccessShareLock on t. If neither is
true yet, wake the point so the worker advances to the next
check. Something like:

I think passing the relation name as the injection point argument
(commit 0fd73cdffc1) makes the injection point fire only while the
worker is scanning the required table, so it can no longer pause on an
earlier relation and the race goes away. Does that work for you?

Yes. I applied the three v3 patches to current master at 6168c65ddca
and ran 002_autoprewarm_lock_yield ten times sequentially. All ten
runs passed. I also checked the server log from the final run and did
not find a crash or an unexpected error.

The relation condition fixes the wrong-relation synchronization
problem I reported.

I moved the LockHasWaiters() fix needed from
https://commitfest.postgresql.org/patch/6732/ here and made it the
0001 patch.

Thanks. Making it 0001 resolves the dependency between the two
changes.

I found two other issues while reviewing v3.

First, 0001 assumes that finding a LOCK in LockMethodLockHash means
that this backend's fast-path lock has already been transferred and
therefore has a PROCLOCK. I don't think that is guaranteed.

For example, backend A can hold a weak relation lock through the fast
path, while backend B acquires the same weak lock through the main
lock table because its fast-path slots are full. In that case, the
LOCK exists because of backend B, but backend A still has no PROCLOCK.
If A calls LockHasWaiters(), 0001 finds the LOCK and then raises:

ERROR: failed to re-find shared proclock object

I think the lookup needs to find both the LOCK and MyProc's PROCLOCK
while holding the partition lock. If either is absent, it should
return false. The locallock pointers should only be assigned after
both objects have been found. A test for this mixed fast-path and
main-table state would also be useful.

Second, the current CFBot run fails in the Linux 32-bit job. The
002_autoprewarm_lock_yield test sets:

shared_buffers = '2GB'

The server then fails during startup with:

FATAL: invalid size -2147483648 for shared memory request for
"Buffer Blocks"

0003 describes the test as manual/local, but it is registered in the
Meson and Make test suites, so CFBot runs it. It either needs a
portable configuration, an early skip on unsupported builds, or
should remain unregistered if it is only intended for manual use.

I couldn't find a better way to make the test deterministic without a
large table, because the injection point can only be attached after
the restart when the worker is already scanning, so a long scan is
what gives the test enough time to attach before the worker finishes.
I'm open to thoughts here.

Please find the attached v3 patches.

Thanks,
Palak

Hi,

On Mon, Sep 7, 2026 at 8:01 AM Palak Chaturvedi
<chaturvedipalak1911@gmail.com> wrote:

I found two other issues while reviewing v3.

Thanks for reviewing it.

First, 0001 assumes that finding a LOCK in LockMethodLockHash means
that this backend's fast-path lock has already been transferred and
therefore has a PROCLOCK. I don't think that is guaranteed.

For example, backend A can hold a weak relation lock through the fast
path, while backend B acquires the same weak lock through the main
lock table because its fast-path slots are full. In that case, the
LOCK exists because of backend B, but backend A still has no PROCLOCK.
If A calls LockHasWaiters(), 0001 finds the LOCK and then raises:

ERROR: failed to re-find shared proclock object

Ah, you are right. Thanks for catching that. I added a "failed to
re-find shared proclock object" error as a test case in the 0003
patch, in case it's useful. It seems like I didn't fully implement
what Robert suggested here:
/messages/by-id/CA+Tgmob3mVc0LgKNtgy-MdDd9KLffzw1X=9qR8UaRmON0xJWNA@mail.gmail.com.
Fixed in the attached v4, which returns false when our proclock isn't
there instead of erroring out.

Second, the current CFBot run fails in the Linux 32-bit job. The
002_autoprewarm_lock_yield test sets:

shared_buffers = '2GB'

The server then fails during startup with:

FATAL: invalid size -2147483648 for shared memory request for
"Buffer Blocks"

0003 describes the test as manual/local, but it is registered in the
Meson and Make test suites, so CFBot runs it. It either needs a
portable configuration, an early skip on unsupported builds, or
should remain unregistered if it is only intended for manual use.

I reduced shared_buffers and relation size to 512MB and about 260MB
respectively and ran the test locally, so I'm not so sure if the CFBot
will be fully happy with it, so I chose to use nocfbot- prefix.

Please find the attached v4 patches.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

Hi,

CommitFest entry 7098 was created for this work, but the mail thread was
detached from it on September 7. The entry now has no thread at all, so
cfbot has nothing to build, and the patch will not show up for anyone
looking for something to review. The entry is still marked Needs review.

Bharath, the history shows you made that change. Should the entry be
closed, or should the thread go back on it? Happy to do either.

Best regards,
Shihao Zhong

Hi,

On Wed, Sep 16, 2026 at 8:04 PM shihao zhong <zhong950419@gmail.com> wrote:

CommitFest entry 7098 was created for this work, but the mail thread was
detached from it on September 7. The entry now has no thread at all, so
cfbot has nothing to build, and the patch will not show up for anyone
looking for something to review. The entry is still marked Needs review.

Bharath, the history shows you made that change. Should the entry be
closed, or should the thread go back on it? Happy to do either.

Thanks, fixed it now. The CF bot picks up the latest patch version for
testing. https://commitfest.postgresql.org/patch/7098/.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com