diff options
| author | Peter Geoghegan <pg@bowt.ie> | 2020-06-11 10:09:40 -0700 |
|---|---|---|
| committer | Peter Geoghegan <pg@bowt.ie> | 2020-06-11 10:09:40 -0700 |
| commit | dd616f2ec1c517da4e6de2801f6386d01d098840 (patch) | |
| tree | 7b5b146f0a688a5013acc2bdf8d87f956cf41f0c /src/backend | |
| parent | 277f154e4f1ccc45b39715d777d8ff6e56fc6c55 (diff) | |
| download | postgresql-dd616f2ec1c517da4e6de2801f6386d01d098840.tar.gz postgresql-dd616f2ec1c517da4e6de2801f6386d01d098840.zip | |
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 dafaa3ef, 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
Diffstat (limited to 'src/backend')
| -rw-r--r-- | src/backend/storage/lmgr/predicate.c | 21 |
1 files changed, 16 insertions, 5 deletions
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index e8390311d03..002d040ba6d 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -3925,6 +3925,10 @@ CheckForSerializableConflictOut(bool visible, Relation relation, * while it is visible to us. The "visible" bool indicates whether the * tuple is visible to us, while HeapTupleSatisfiesVacuum checks what else * is going on with it. + * + * In the event of a concurrently inserted tuple that also happens to have + * been concurrently updated (by a separate transaction), the xmin of the + * tuple will be used -- not the updater's xid. */ htsvResult = HeapTupleSatisfiesVacuum(tuple, TransactionXmin, buffer); switch (htsvResult) @@ -3935,17 +3939,24 @@ CheckForSerializableConflictOut(bool visible, Relation relation, xid = HeapTupleHeaderGetXmin(tuple->t_data); break; case HEAPTUPLE_RECENTLY_DEAD: - if (!visible) - return; - xid = HeapTupleHeaderGetUpdateXid(tuple->t_data); - break; case HEAPTUPLE_DELETE_IN_PROGRESS: - xid = HeapTupleHeaderGetUpdateXid(tuple->t_data); + if (visible) + xid = HeapTupleHeaderGetUpdateXid(tuple->t_data); + else + xid = HeapTupleHeaderGetXmin(tuple->t_data); + + if (TransactionIdPrecedes(xid, TransactionXmin)) + { + /* This is like the HEAPTUPLE_DEAD case */ + Assert(!visible); + return; + } break; case HEAPTUPLE_INSERT_IN_PROGRESS: xid = HeapTupleHeaderGetXmin(tuple->t_data); break; case HEAPTUPLE_DEAD: + Assert(!visible); return; default: |