Re: FIX: BUG #19687: ALTER SEQUENCE missing lock

From: Alexandre Felipe <o(dot)alexandre(dot)felipe(at)gmail(dot)com>
To: Andrew Krylosov <krylosov(dot)andrew(at)gmail(dot)com>
Cc: Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: FIX: BUG #19687: ALTER SEQUENCE missing lock
Date: 2026-09-27 06:36:04
Message-ID: CAE8JnxN3+_hEQDQcw08cNCW6VcXArNa2dRYJ68bPxEprk2CRsA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Sep 15, 2026 at 9:00 AM Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
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 <krylosov(dot)andrew(at)gmail(dot)com>
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/19687-dd094472b0c48afb@postgresql.org
>
> 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
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thom Brown 2026-09-27 08:03:15 Re: REPACK (CONCURRENTLY) can lose data in pg_dump output
Previous Message Chao Li 2026-09-27 06:17:07 Re: pg_resetwal with replication slot (17.11)