| 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
>
| 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) |