On Tue, 15 Sept 2026 at 02:10, Bharath Rupireddy <[email protected]> wrote: > > Hi, > > On Mon, Sep 7, 2026 at 8:01 AM Palak Chaturvedi > <[email protected]> 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: > https://postgr.es/m/CA%2BTgmob3mVc0LgKNtgy-MdDd9KLffzw1X%3D9qR8UaRmON0xJWNA%40mail.gmail.com. > Fixed in the attached v4, which returns false when our proclock isn't > there instead of erroring out.
Thanks. I tested v4 on master at 6168c65ddca with assertions and injection points enabled. The pg_prewarm suite passes here. There is another case to handle: the same backend can hold AccessShareLock through the fast path and ShareUpdateExclusiveLock through the main table on the same relation. In that case, our proclock exists, but its holdMask does not contain AccessShareLock. I reproduced this using a small SQL-callable C wrapper around LockHasWaiters(). In one backend: BEGIN; LOCK TABLE probe_target IN ACCESS SHARE MODE; LOCK TABLE probe_target IN SHARE UPDATE EXCLUSIVE MODE; pg_locks confirms that AccessShareLock is fast-path and ShareUpdateExclusiveLock is not. Calling LockHasWaiters() for AccessShareLock finds our proclock, so the new if (!proclock) check does not return. It then reaches the existing holdMask check and reports: WARNING: you don't own a lock of type AccessShareLock That branch calls RemoveLocalLock(). LockHeldByMe() subsequently returns false, although pg_locks still shows the fast-path lock as granted. After ROLLBACK, that lock remains held. Could the new fast-path lookup branch check the requested mode too? if (!proclock || !(proclock->holdMask & LOCKBIT_ON(lockmode))) If the mode is absent, our lock has not been transferred, so this branch can release the partition lock and return false without touching the local ownership record. With that change, the same reproducer retains ownership, emits no warning, and leaves no relation locks after rollback. The pg_prewarm suite still passes. A test for this same-backend case would be useful alongside the other-backend case added in v4. > > > > > > 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. > The smaller configuration works here, but both scenarios still attach the injection point after restarting the server. The worker can finish before attachment. I forced that ordering by waiting for the prewarm completion log after the first restart, before attaching the point. The worker completed 33635/33635 blocks, attachment succeeded, and wait_for_event() timed out. This was a deliberately forced ordering, not a spontaneous failure of the unmodified test. Could we arrange for attachment before the worker starts scanning? That would remove the timing dependency and allow a smaller table. > Please find the attached v4 patches. Thanks, Palak
