From: Alexander Lakhin <exclusion@gmail.com>
To: pgsql-bugs@lists.postgresql.org
Cc: Andres Freund <andres@anarazel.de>
Subject: Re: BUG #17893: Assert failed in heap_update()/_delete() when FK modiified by RI trigger in non-read-committed xact
Date: Sat, 2 Sep 2023 16:00:00 +0300
Message-ID: <b77078c0-090c-0f0e-6f13-3bb7daca93ee@gmail.com> (raw)
In-Reply-To: <17893-35847009eec517b5@postgresql.org>
References: <17893-35847009eec517b5@postgresql.org>
12.04.2023 15:00, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference: 17893
>
> The following script:
> ...
>
> triggers an assertion failure:
I've come to this assertion failure again, this time from another side, so I
have one reason more to move forward to the issue fix.
First, I've found that two instances of "additional check for
transaction-snapshot mode RI updates" in head_update() and heap_delete()
are not covered by the existing regression tests at all, so I would like to
propose an addition for the fk-snapshot test (see attached). It increases
coverage and causes the assertion failures in both functions.
Second, it looks like Assert(!(tp.t_data->t_infomask & HEAP_XMAX_INVALID))
was added to make sure that tmfd->xmax = HeapTupleHeaderGetUpdateXid(tp.t_data);
(which ends up with just returning t_xmax) returns valid xmax. Really, in the
problematic case we get zero in tmfd->xmax. But ExecDelete(), ExecUpdate() do:
...
switch (result)
...
case TM_Updated:
...
if (IsolationUsesXactSnapshot())
ereport(ERROR,
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
errmsg("could not serialize access due to concurrent update")));
before any processing of tmfd. So it seems like the invalid xmax is not
an issue in this case.
Thus, maybe
Assert((!(tp.t_data->t_infomask & HEAP_XMAX_INVALID))
should be changed to
Assert(!(tp.t_data->t_infomask & HEAP_XMAX_INVALID) ||
(result == TM_Updated && crosscheck != InvalidSnapshot));
Third, with this change applied I immediately got a failure of the next
assert in heap_delete():
Assert(!ItemPointerEquals(&oldtup.t_self, &oldtup.t_data->t_ctid));
which was introduced by 5db6df0c0.
(By the way, I've noticed a typo brought by that commit:
t_xmax, and, if possible, and, if possible, t_cmax
)
It's not clear to me yet, which anomalous condition that Assert should
detect, specifically in heap_update(), inside this branch:
/* Perform additional check for transaction-snapshot mode RI updates */
if (!HeapTupleSatisfiesVisibility(&oldtup, crosscheck, buffer))
{
result = TM_Updated;
Assert(!ItemPointerEquals(&oldtup.t_self, &oldtup.t_data->t_ctid));
}
Andres, could you shed some light on this, please?
Best regards,
Alexander
Attachments:fk-snapshot.patch (text/x-patch, 3.2K) downloaddiff --git a/src/test/isolation/expected/fk-snapshot.out b/src/test/isolation/expected/fk-snapshot.outindex 5faf80d6ce..bdd26bac6c 100644--- a/src/test/isolation/expected/fk-snapshot.out+++ b/src/test/isolation/expected/fk-snapshot.out@@ -122,3 +122,25 @@ a
1
(1 row)
++starting permutation: s2ip2 s1brr s1ifp2 s2brr s2dp2 s1c s2c+step s2ip2: INSERT INTO pk_noparted VALUES (2);+step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ;+step s1ifp2: INSERT INTO fk_parted_pk VALUES (2);+step s2brr: BEGIN ISOLATION LEVEL REPEATABLE READ;+step s2dp2: DELETE FROM pk_noparted WHERE a = 2; <waiting ...>+step s1c: COMMIT;+step s2dp2: <... completed>+ERROR: could not serialize access due to concurrent update+step s2c: COMMIT;++starting permutation: s2ip2 s1brr s1ifn2 s2brr s2dp2 s1c s2c+step s2ip2: INSERT INTO pk_noparted VALUES (2);+step s1brr: BEGIN ISOLATION LEVEL REPEATABLE READ;+step s1ifn2: INSERT INTO fk_noparted_sn VALUES (2);+step s2brr: BEGIN ISOLATION LEVEL REPEATABLE READ;+step s2dp2: DELETE FROM pk_noparted WHERE a = 2; <waiting ...>+step s1c: COMMIT;+step s2dp2: <... completed>+ERROR: could not serialize access due to concurrent update+step s2c: COMMIT;diff --git a/src/test/isolation/specs/fk-snapshot.spec b/src/test/isolation/specs/fk-snapshot.specindex 378507fbc3..9fad57e768 100644--- a/src/test/isolation/specs/fk-snapshot.spec+++ b/src/test/isolation/specs/fk-snapshot.spec@@ -13,6 +13,11 @@ setup
CREATE TABLE fk_noparted (
a int REFERENCES fk_parted_pk ON DELETE NO ACTION INITIALLY DEFERRED
);
++ CREATE TABLE fk_noparted_sn (+ a int REFERENCES pk_noparted ON DELETE SET NULL+ );+
INSERT INTO pk_noparted VALUES (1);
INSERT INTO fk_parted_pk VALUES (1);
INSERT INTO fk_noparted VALUES (1);
@@ -20,7 +25,7 @@ setup
teardown
{
- DROP TABLE pk_noparted, fk_parted_pk, fk_noparted;+ DROP TABLE pk_noparted, fk_parted_pk, fk_noparted, fk_noparted_sn;
}
session s1
@@ -28,6 +33,7 @@ step s1brr { BEGIN ISOLATION LEVEL REPEATABLE READ; }
step s1brc { BEGIN ISOLATION LEVEL READ COMMITTED; }
step s1ifp2 { INSERT INTO fk_parted_pk VALUES (2); }
step s1ifp1 { INSERT INTO fk_parted_pk VALUES (1); }
+step s1ifn2 { INSERT INTO fk_noparted_sn VALUES (2); }
step s1dfp { DELETE FROM fk_parted_pk WHERE a = 1; }
step s1c { COMMIT; }
step s1sfp { SELECT * FROM fk_parted_pk; }
@@ -38,6 +44,7 @@ session s2
step s2brr { BEGIN ISOLATION LEVEL REPEATABLE READ; }
step s2brc { BEGIN ISOLATION LEVEL READ COMMITTED; }
step s2ip2 { INSERT INTO pk_noparted VALUES (2); }
+step s2dp2 { DELETE FROM pk_noparted WHERE a = 2; }
step s2ifn2 { INSERT INTO fk_noparted VALUES (2); }
step s2c { COMMIT; }
step s2sfp { SELECT * FROM fk_parted_pk; }
@@ -59,3 +66,11 @@ permutation s1brc s2brc s2ip2 s1sp s2c s1sp s1ifp2 s2brc s2sfp s1c s1sfp s2ifn2
# the same no matter the snapshot mode
permutation s1brr s1dfp s1ifp1 s1c s1sfn
permutation s1brc s1dfp s1ifp1 s1c s1sfn
++# trying to delete a row through DELETE CASCADE, whilst that row is deleted+# in a concurrent transaction+permutation s2ip2 s1brr s1ifp2 s2brr s2dp2 s1c s2c++# trying to update a row through DELETE SET NULL, whilst that row is deleted+# in a concurrent transaction+permutation s2ip2 s1brr s1ifn2 s2brr s2dp2 s1c s2c
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: exclusion@gmail.com, pgsql-bugs@lists.postgresql.org, andres@anarazel.de
Subject: Re: BUG #17893: Assert failed in heap_update()/_delete() when FK modiified by RI trigger in non-read-committed xact
In-Reply-To: <b77078c0-090c-0f0e-6f13-3bb7daca93ee@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox