Skip to content

Commit 5940ffb

Browse files
petergeogheganpetergeoghegan
committed
Avoid update conflict out serialization anomalies.
SSI's HeapCheckForSerializableConflictOut() test failed to correctly handle conditions involving a concurrently inserted tuple which is later concurrently updated by a separate transaction . A SELECT statement that called HeapCheckForSerializableConflictOut() could end up using the same XID (updater's XID) for both the original tuple, and the successor tuple, missing the XID of the xact that created the original tuple entirely. This only happened when neither tuple from the chain was visible to the transaction's MVCC snapshot. The observable symptoms of this bug were subtle. A pair of transactions could commit, with the later transaction failing to observe the effects of the earlier transaction (because of the confusion created by the update to the non-visible row). This bug dates all the way back to commit dafaa3e, which added SSI. To fix, make sure that we check the xmin of concurrently inserted tuples that happen to also have been updated concurrently. Author: Peter Geoghegan Reported-By: Kyle Kingsbury Reviewed-By: Thomas Munro Discussion: https://postgr.es/m/db7b729d-0226-d162-a126-8a8ab2dc4443@jepsen.io Backpatch: All supported versions
1 parent d9fa17a commit 5940ffb

File tree

4 files changed

+98
-5
lines changed

4 files changed

+98
-5
lines changed

‎src/backend/access/heap/heapam.c

+16-5
Original file line numberDiff line numberDiff line change
@@ -9007,6 +9007,10 @@ HeapCheckForSerializableConflictOut(bool visible, Relation relation,
90079007
* while it is visible to us. The "visible" bool indicates whether the
90089008
* tuple is visible to us, while HeapTupleSatisfiesVacuum checks what else
90099009
* is going on with it.
9010+
*
9011+
* In the event of a concurrently inserted tuple that also happens to have
9012+
* been concurrently updated (by a separate transaction), the xmin of the
9013+
* tuple will be used -- not the updater's xid.
90109014
*/
90119015
htsvResult = HeapTupleSatisfiesVacuum(tuple, TransactionXmin, buffer);
90129016
switch (htsvResult)
@@ -9017,17 +9021,24 @@ HeapCheckForSerializableConflictOut(bool visible, Relation relation,
90179021
xid = HeapTupleHeaderGetXmin(tuple->t_data);
90189022
break;
90199023
case HEAPTUPLE_RECENTLY_DEAD:
9020-
if (!visible)
9021-
return;
9022-
xid = HeapTupleHeaderGetUpdateXid(tuple->t_data);
9023-
break;
90249024
case HEAPTUPLE_DELETE_IN_PROGRESS:
9025-
xid = HeapTupleHeaderGetUpdateXid(tuple->t_data);
9025+
if (visible)
9026+
xid = HeapTupleHeaderGetUpdateXid(tuple->t_data);
9027+
else
9028+
xid = HeapTupleHeaderGetXmin(tuple->t_data);
9029+
9030+
if (TransactionIdPrecedes(xid, TransactionXmin))
9031+
{
9032+
/* This is like the HEAPTUPLE_DEAD case */
9033+
Assert(!visible);
9034+
return;
9035+
}
90269036
break;
90279037
case HEAPTUPLE_INSERT_IN_PROGRESS:
90289038
xid = HeapTupleHeaderGetXmin(tuple->t_data);
90299039
break;
90309040
case HEAPTUPLE_DEAD:
9041+
Assert(!visible);
90319042
return;
90329043
default:
90339044

‎src/test/isolation/expected/update-conflict-out.out

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
Parsed test spec with 3 sessions
2+
3+
starting permutation: foo_select bar_insert foo_insert foo_commit trouble_update bar_select bar_commit trouble_abort
4+
step foo_select: SELECT * FROM txn0 WHERE id = 42;
5+
id val
6+
7+
step bar_insert: INSERT INTO txn0 SELECT 42, 'bar_insert';
8+
step foo_insert: INSERT INTO txn1 SELECT 7, 'foo_insert';
9+
step foo_commit: COMMIT;
10+
step trouble_update: UPDATE txn1 SET val = 'add physical version for "bar_select"' WHERE id = 7;
11+
step bar_select: SELECT * FROM txn1 WHERE id = 7;
12+
ERROR: could not serialize access due to read/write dependencies among transactions
13+
step bar_commit: COMMIT;
14+
step trouble_abort: ABORT;
15+
16+
starting permutation: foo_select bar_insert foo_insert foo_commit trouble_delete bar_select bar_commit trouble_abort
17+
step foo_select: SELECT * FROM txn0 WHERE id = 42;
18+
id val
19+
20+
step bar_insert: INSERT INTO txn0 SELECT 42, 'bar_insert';
21+
step foo_insert: INSERT INTO txn1 SELECT 7, 'foo_insert';
22+
step foo_commit: COMMIT;
23+
step trouble_delete: DELETE FROM txn1 WHERE id = 7;
24+
step bar_select: SELECT * FROM txn1 WHERE id = 7;
25+
ERROR: could not serialize access due to read/write dependencies among transactions
26+
step bar_commit: COMMIT;
27+
step trouble_abort: ABORT;

‎src/test/isolation/isolation_schedule

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ test: two-ids
1818
test: multiple-row-versions
1919
test: index-only-scan
2020
test: predicate-lock-hot-tuple
21+
test: update-conflict-out
2122
test: deadlock-simple
2223
test: deadlock-hard
2324
test: deadlock-soft

‎src/test/isolation/specs/update-conflict-out.spec

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# Test for interactions between SSI's "conflict out" handling for heapam and
2+
# concurrently updated tuple
3+
#
4+
# See bug report:
5+
# https://postgr.es/m/db7b729d-0226-d162-a126-8a8ab2dc4443%40jepsen.io
6+
7+
setup
8+
{
9+
CREATE TABLE txn0(id int4 PRIMARY KEY, val TEXT);
10+
CREATE TABLE txn1(id int4 PRIMARY KEY, val TEXT);
11+
}
12+
13+
teardown
14+
{
15+
DROP TABLE txn0;
16+
DROP TABLE txn1;
17+
}
18+
19+
session "foo"
20+
setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; }
21+
step "foo_select" { SELECT * FROM txn0 WHERE id = 42; }
22+
step "foo_insert" { INSERT INTO txn1 SELECT 7, 'foo_insert'; }
23+
step "foo_commit" { COMMIT; }
24+
25+
session "bar"
26+
setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; }
27+
step "bar_select" { SELECT * FROM txn1 WHERE id = 7; }
28+
step "bar_insert" { INSERT INTO txn0 SELECT 42, 'bar_insert'; }
29+
step "bar_commit" { COMMIT; }
30+
31+
# This session creates the conditions that confused bar's "conflict out"
32+
# handling in old releases affected by bug:
33+
session "trouble"
34+
setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; }
35+
step "trouble_update" { UPDATE txn1 SET val = 'add physical version for "bar_select"' WHERE id = 7; }
36+
step "trouble_delete" { DELETE FROM txn1 WHERE id = 7; }
37+
step "trouble_abort" { ABORT; }
38+
39+
permutation "foo_select"
40+
"bar_insert"
41+
"foo_insert" "foo_commit"
42+
"trouble_update" # Updates tuple...
43+
"bar_select" # Should observe one distinct XID per version
44+
"bar_commit" # "bar" should fail here at the latest
45+
"trouble_abort"
46+
47+
# Same as above, but "trouble" session DELETEs this time around
48+
permutation "foo_select"
49+
"bar_insert"
50+
"foo_insert" "foo_commit"
51+
"trouble_delete" # Deletes tuple...
52+
"bar_select" # Should observe foo's XID
53+
"bar_commit" # "bar" should fail here at the latest
54+
"trouble_abort"

0 commit comments

Comments
 (0)