Re: Make pg_prewarm, autoprewarm yield for waiting DDL

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(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Jul 14, 2026 at 8:21 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> >
> > On Wed, Mar 25, 2026 at 5:32 PM SATYANARAYANA NARLAPURAM
> > <satyanarlapuram(at)gmail(dot)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