| From: | Palak Chaturvedi <chaturvedipalak1911(at)gmail(dot)com> |
|---|---|
| To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
| Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Robert Haas <robertmhaas(at)gmail(dot)com>, SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Make pg_prewarm, autoprewarm yield for waiting DDL |
| Date: | 2026-09-07 15:01:20 |
| Message-ID: | CALfch1809tLV-M5n=pC6fSPE+GyZQQuXEnidT+fsGiG=JyXy1A@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Bharath,
On Wed, 2 Sept 2026 at 16:23, Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Mon, Aug 17, 2026 at 6:44 AM Palak Chaturvedi
> <chaturvedipalak1911(at)gmail(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Osama Abdul Qader | 2026-09-07 15:16:50 | Re: Prevent object capture in CREATE/ALTER EXTENSION scripts |
| Previous Message | Grigorev Jurij | 2026-09-07 14:52:59 | Missing dshash cleanup in pgstat_read_statsfile() after OOM |