From 7aa4fb592530b74bf05f62c52b736ee3910691b9 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Fri, 12 Jun 2020 10:44:32 +1200 Subject: [PATCH] Improve comments for [Heap]CheckForSerializableConflictOut(). Rewrite the documentation of these functions, in light of recent bug fix commit 5940ffb2. Back-patch to 13 where the check-for-conflict-out code was split up into AM-specific and generic parts, and new documentation was added that now looked wrong. Reviewed-by: Peter Geoghegan Discussion: https://postgr.es/m/db7b729d-0226-d162-a126-8a8ab2dc4443%40jepsen.io --- src/backend/access/heap/heapam.c | 12 +++++------- src/backend/storage/lmgr/predicate.c | 15 +++++++++------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 9756bf98bd..537913d1bb 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -8975,15 +8975,13 @@ heap_mask(char *pagedata, BlockNumber blkno) /* * HeapCheckForSerializableConflictOut - * We are reading a tuple which has been modified. If it is visible to - * us but has been deleted, that indicates a rw-conflict out. If it's - * not visible and was created by a concurrent (overlapping) - * serializable transaction, that is also a rw-conflict out, + * We are reading a tuple. If it's not visible, there may be a + * rw-conflict out with the inserter. Otherwise, if it is visible to us + * but has been deleted, there may be a rw-conflict out with the deleter. * * We will determine the top level xid of the writing transaction with which - * we may be in conflict, and check for overlap with our own transaction. - * If the transactions overlap (i.e., they cannot see each other's writes), - * then we have a conflict out. + * we may be in conflict, and ask CheckForSerializableConflictOut() to check + * for overlap with our own transaction. * * This function should be called just about anywhere in heapam.c where a * tuple has been read. The caller must hold at least a shared lock on the diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index ba93fb199d..d24919f76b 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -4037,13 +4037,16 @@ CheckForSerializableConflictOutNeeded(Relation relation, Snapshot snapshot) /* * CheckForSerializableConflictOut - * A table AM is reading a tuple that has been modified. After determining - * that it is visible to us, it should call this function with the top - * level xid of the writing transaction. + * A table AM is reading a tuple that has been modified. If it determines + * that the tuple version it is reading is not visible to us, it should + * pass in the top level xid of the transaction that created it. + * Otherwise, if it determines that it is visible to us but it has been + * deleted or there is a newer version available due to an update, it + * should pass in the top level xid of the modifying transaction. * - * This function will check for overlap with our own transaction. If the - * transactions overlap (i.e., they cannot see each other's writes), then we - * have a conflict out. + * This function will check for overlap with our own transaction. If the given + * xid is also serializable and the transactions overlap (i.e., they cannot see + * each other's writes), then we have a conflict out. */ void CheckForSerializableConflictOut(Relation relation, TransactionId xid, Snapshot snapshot)