On Tue, Sep 15, 2026 at 9:00 AM Ayush Tiwari <[email protected]> wrote:
> I posted about the same bug on pgsql-bugs yesterday [2] (quoted below), > so maybe it is better to keep this in one thread. > Hi Ayush, do you want to take care of this, since you already did some good work on it? Thank you for your review Andrew On Sat, Sep 26, 2026 at 11:46 PM Andrew Krylosov <[email protected]> wrote: > Hi, > > I applied v1 on top of 3c5d9d914f and built it with cassert. regress > and isolation pass, and Alexander's script no longer fails for me > (200 iterations, with and without the forced parallel plan; on HEAD > it fails within 50). > > However, the lock upgrade in v1 is exactly what Ayush described > upthread, not a lock inside RelationSetNewRelfilenumber(): > > + LockRelationOid(relid, AccessExclusiveLock); > > At this point we already hold ShareRowExclusiveLock, so a transaction > that has read the sequence and then calls nextval() deadlocks with us: > > s2: BEGIN; SELECT last_value FROM seq1; > s1: BEGIN; ALTER SEQUENCE seq1 AS int; -- waits > s2: SELECT nextval('seq1'); > ERROR: deadlock detected > > The same deadlock occurs if s2 runs ALTER SEQUENCE instead of > nextval(). On HEAD, s2 waits for s1 in both cases. > > Taking AccessExclusiveLock at the initial RangeVarGetRelidExtended() > call avoids these deadlocks: s1 waits for s2, and s2 can finish. > This also fixes the reported failures. ResetSequence() and > SequenceChangePersistence() already use this lock mode. > > This change was also discussed in the bug thread: > https://postgr.es/m/[email protected] > > The downside is that ALTER SEQUENCE ... OWNED BY alone would also > block readers, even though it doesn't rewrite the sequence. We could > choose the lock mode from the option list up front to preserve the > current behavior for that case, but I'm not sure the extra complexity > is worth it. > > AccessExclusiveLock also covers hot standby, since it is WAL-logged. > On HEAD, a standby query that reads the sequence across the replay of > the ALTER's commit fails with > > ERROR: could not open file "base/5/16384": No such file or directory > > while with the stronger lock it gets the usual recovery conflict. > > Also I think the patch needs a test. An isolation permutation in > sequence-ddl.spec with a cursor open on the sequence works > deterministically: on HEAD the ALTER doesn't wait and the following > FETCH silently returns zero rows, with the fix the ALTER waits. > A SELECT-then-nextval() permutation would catch the deadlock above. > > The new comment isn't quite accurate: RelationSetNewRelfilenumber() > doesn't destroy the old file, it is dropped at commit while other > backends may still be reading it. > > Best regards, > Andrew Krylosov >
