On Wed, Jul 31, 2019 at 12:37 PM Ashwin Agrawal <aagra...@pivotal.io> wrote:
> > On Wed, Jul 31, 2019 at 10:55 AM Andres Freund <and...@anarazel.de> wrote: > >> Hi, >> >> On 2019-07-31 10:42:50 -0700, Ashwin Agrawal wrote: >> > On Tue, Jul 30, 2019 at 2:58 PM Thomas Munro <thomas.mu...@gmail.com> >> wrote: >> > >> > > On Tue, Jun 25, 2019 at 6:02 AM Andres Freund <and...@anarazel.de> >> wrote: >> > > > > - CheckForSerializableConflictOut() no more takes HeapTuple nor >> > > > > buffer, instead just takes xid. Push heap specific parts from >> > > > > CheckForSerializableConflictOut() into its own function >> > > > > HeapCheckForSerializableConflictOut() which calls >> > > > > CheckForSerializableConflictOut(). The alternative option could >> be >> > > > > CheckForSerializableConflictOut() take callback function and >> > > > > callback arguments, which gets called if required after >> performing >> > > > > prechecks. Though currently I fell AM having its own wrapper to >> > > > > perform AM specific task and then calling >> > > > > CheckForSerializableConflictOut() is fine. >> > > > >> > > > I think it's right to move the xid handling out of >> > > > CheckForSerializableConflictOut(). But I think we also ought to >> move the >> > > > subtransaction handling out of the function - e.g. zheap doesn't >> > > > want/need that. >> > > >> > > Thoughts on this Ashwin? >> > > >> > >> > I think the only part its doing for sub-transaction is >> > SubTransGetTopmostTransaction(xid). If xid passed to this function is >> > already top most transaction which is case for zheap and zedstore, then >> > there is no downside to keeping that code here in common place. >> >> Well, it's far from a cheap function. It'll do unnecessary on-disk >> lookups in many cases. I'd call that quite a downside. >> > > Okay, agree, its costly function and better to avoid the call if possible. > > Instead of moving the handling out of the function, how do feel about > adding boolean isTopTransactionId argument to function > CheckForSerializableConflictOut(). The AMs, which implicitly know, only > pass top transaction Id to this function, can pass true and avoid the > function call to SubTransGetTopmostTransaction(xid). With this > subtransaction code remains in generic place and AMs intending to use it > continue to leverage the common code, plus explicitly clarifies the > behavior as well. > Added argument to function to make the subtransaction handling optional in attached v2 of patch.
From 4ca67592f34b63cf80247068a128407d800c62a6 Mon Sep 17 00:00:00 2001 From: Ashwin Agrawal <aagra...@pivotal.io> Date: Wed, 31 Jul 2019 13:53:52 -0700 Subject: [PATCH v2] Remove HeapTuple dependency for predicate locking functions. Following changes help to make predicate locking functions generic and not tied to particular AM. - PredicateLockTuple() is renamed to PredicateLockTID(). It takes ItemPointer and inserting transaction id now instead of HeapTuple. - CheckForSerializableConflictIn() takes blocknum instead of buffer - CheckForSerializableConflictOut() no more takes HeapTuple nor buffer --- src/backend/access/gin/ginbtree.c | 2 +- src/backend/access/gin/ginfast.c | 2 +- src/backend/access/gin/gininsert.c | 4 +- src/backend/access/gist/gist.c | 2 +- src/backend/access/hash/hashinsert.c | 2 +- src/backend/access/heap/heapam.c | 104 ++++++++++++++++-- src/backend/access/heap/heapam_handler.c | 7 +- src/backend/access/index/indexam.c | 4 +- src/backend/access/nbtree/nbtinsert.c | 4 +- src/backend/storage/lmgr/predicate.c | 134 ++++++++--------------- src/include/access/heapam.h | 2 + src/include/storage/predicate.h | 10 +- 12 files changed, 162 insertions(+), 115 deletions(-) diff --git a/src/backend/access/gin/ginbtree.c b/src/backend/access/gin/ginbtree.c index 11a8ed7bbc2..e795375495b 100644 --- a/src/backend/access/gin/ginbtree.c +++ b/src/backend/access/gin/ginbtree.c @@ -89,7 +89,7 @@ ginFindLeafPage(GinBtree btree, bool searchMode, stack->predictNumber = 1; if (rootConflictCheck) - CheckForSerializableConflictIn(btree->index, NULL, stack->buffer); + CheckForSerializableConflictIn(btree->index, NULL, btree->rootBlkno); for (;;) { diff --git a/src/backend/access/gin/ginfast.c b/src/backend/access/gin/ginfast.c index 439a91b3e61..d7b52476817 100644 --- a/src/backend/access/gin/ginfast.c +++ b/src/backend/access/gin/ginfast.c @@ -246,7 +246,7 @@ ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector) * tree, so it conflicts with all serializable scans. All scans acquire a * predicate lock on the metabuffer to represent that. */ - CheckForSerializableConflictIn(index, NULL, metabuffer); + CheckForSerializableConflictIn(index, NULL, GIN_METAPAGE_BLKNO); if (collector->sumsize + collector->ntuples * sizeof(ItemIdData) > GinListPageSize) { diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c index 55eab146173..046a20a3d41 100644 --- a/src/backend/access/gin/gininsert.c +++ b/src/backend/access/gin/gininsert.c @@ -221,7 +221,7 @@ ginEntryInsert(GinState *ginstate, return; } - CheckForSerializableConflictIn(ginstate->index, NULL, stack->buffer); + CheckForSerializableConflictIn(ginstate->index, NULL, BufferGetBlockNumber(stack->buffer)); /* modify an existing leaf entry */ itup = addItemPointersToLeafTuple(ginstate, itup, items, nitem, buildStats, stack->buffer); @@ -230,7 +230,7 @@ ginEntryInsert(GinState *ginstate, } else { - CheckForSerializableConflictIn(ginstate->index, NULL, stack->buffer); + CheckForSerializableConflictIn(ginstate->index, NULL, BufferGetBlockNumber(stack->buffer)); /* no match, so construct a new leaf entry */ itup = buildFreshLeafTuple(ginstate, attnum, key, category, items, nitem, buildStats, stack->buffer); diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index e9ca4b82527..4db54a42761 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -1263,7 +1263,7 @@ gistinserttuples(GISTInsertState *state, GISTInsertStack *stack, * Check for any rw conflicts (in serializable isolation level) just * before we intend to modify the page */ - CheckForSerializableConflictIn(state->r, NULL, stack->buffer); + CheckForSerializableConflictIn(state->r, NULL, BufferGetBlockNumber(stack->buffer)); /* Insert the tuple(s) to the page, splitting the page if necessary */ is_split = gistplacetopage(state->r, state->freespace, giststate, diff --git a/src/backend/access/hash/hashinsert.c b/src/backend/access/hash/hashinsert.c index 5321762d5ea..e3fb47f9e3d 100644 --- a/src/backend/access/hash/hashinsert.c +++ b/src/backend/access/hash/hashinsert.c @@ -88,7 +88,7 @@ restart_insert: &usedmetap); Assert(usedmetap != NULL); - CheckForSerializableConflictIn(rel, NULL, buf); + CheckForSerializableConflictIn(rel, NULL, BufferGetBlockNumber(buf)); /* remember the primary bucket buffer to release the pin on it at end. */ bucket_buf = buf; diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 94309949fac..8475c4b0168 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -40,6 +40,7 @@ #include "access/multixact.h" #include "access/parallel.h" #include "access/relscan.h" +#include "access/subtrans.h" #include "access/sysattr.h" #include "access/tableam.h" #include "access/transam.h" @@ -446,7 +447,7 @@ heapgetpage(TableScanDesc sscan, BlockNumber page) else valid = HeapTupleSatisfiesVisibility(&loctup, snapshot, buffer); - CheckForSerializableConflictOut(valid, scan->rs_base.rs_rd, + HeapCheckForSerializableConflictOut(valid, scan->rs_base.rs_rd, &loctup, buffer, snapshot); if (valid) @@ -668,7 +669,7 @@ heapgettup(HeapScanDesc scan, snapshot, scan->rs_cbuf); - CheckForSerializableConflictOut(valid, scan->rs_base.rs_rd, + HeapCheckForSerializableConflictOut(valid, scan->rs_base.rs_rd, tuple, scan->rs_cbuf, snapshot); @@ -1477,9 +1478,10 @@ heap_fetch(Relation relation, valid = HeapTupleSatisfiesVisibility(tuple, snapshot, buffer); if (valid) - PredicateLockTuple(relation, tuple, snapshot); + PredicateLockTID(relation, &(tuple->t_self), snapshot, + HeapTupleHeaderGetXmin(tuple->t_data)); - CheckForSerializableConflictOut(valid, relation, tuple, buffer, snapshot); + HeapCheckForSerializableConflictOut(valid, relation, tuple, buffer, snapshot); LockBuffer(buffer, BUFFER_LOCK_UNLOCK); @@ -1613,7 +1615,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer, /* If it's visible per the snapshot, we must return it */ valid = HeapTupleSatisfiesVisibility(heapTuple, snapshot, buffer); - CheckForSerializableConflictOut(valid, relation, heapTuple, + HeapCheckForSerializableConflictOut(valid, relation, heapTuple, buffer, snapshot); /* reset to original, non-redirected, tid */ heapTuple->t_self = *tid; @@ -1621,7 +1623,8 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer, if (valid) { ItemPointerSetOffsetNumber(tid, offnum); - PredicateLockTuple(relation, heapTuple, snapshot); + PredicateLockTID(relation, &(heapTuple)->t_self, snapshot, + HeapTupleHeaderGetXmin(heapTuple->t_data)); if (all_dead) *all_dead = false; return true; @@ -1755,7 +1758,7 @@ heap_get_latest_tid(TableScanDesc sscan, * candidate. */ valid = HeapTupleSatisfiesVisibility(&tp, snapshot, buffer); - CheckForSerializableConflictOut(valid, relation, &tp, buffer, snapshot); + HeapCheckForSerializableConflictOut(valid, relation, &tp, buffer, snapshot); if (valid) *tid = ctid; @@ -1910,7 +1913,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid, * lock "gaps" as index page locks do. So we don't need to specify a * buffer when making the call, which makes for a faster check. */ - CheckForSerializableConflictIn(relation, NULL, InvalidBuffer); + CheckForSerializableConflictIn(relation, NULL, InvalidBlockNumber); /* NO EREPORT(ERROR) from here till changes are logged */ START_CRIT_SECTION(); @@ -2164,7 +2167,7 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples, * lock "gaps" as index page locks do. So we don't need to specify a * buffer when making the call, which makes for a faster check. */ - CheckForSerializableConflictIn(relation, NULL, InvalidBuffer); + CheckForSerializableConflictIn(relation, NULL, InvalidBlockNumber); ndone = 0; while (ndone < ntuples) @@ -2355,7 +2358,7 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples, * lock "gaps" as index page locks do. So we don't need to specify a * buffer when making the call. */ - CheckForSerializableConflictIn(relation, NULL, InvalidBuffer); + CheckForSerializableConflictIn(relation, NULL, InvalidBlockNumber); /* * If tuples are cachable, mark them for invalidation from the caches in @@ -2669,7 +2672,7 @@ l1: * being visible to the scan (i.e., an exclusive buffer content lock is * continuously held from this point until the tuple delete is visible). */ - CheckForSerializableConflictIn(relation, &tp, buffer); + CheckForSerializableConflictIn(relation, tid, BufferGetBlockNumber(buffer)); /* replace cid with a combo cid if necessary */ HeapTupleHeaderAdjustCmax(tp.t_data, &cid, &iscombo); @@ -3584,7 +3587,7 @@ l2: * will include checking the relation level, there is no benefit to a * separate check for the new tuple. */ - CheckForSerializableConflictIn(relation, &oldtup, buffer); + CheckForSerializableConflictIn(relation, otid, BufferGetBlockNumber(buffer)); /* * At this point newbuf and buffer are both pinned and locked, and newbuf @@ -9044,3 +9047,80 @@ 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 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. + * + * 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 + * buffer, because this function might set hint bits on the tuple. There is + * currently no known reason to call this function from an index AM. + */ +void +HeapCheckForSerializableConflictOut(bool visible, Relation relation, + HeapTuple tuple, Buffer buffer, + Snapshot snapshot) +{ + TransactionId xid; + HTSV_Result htsvResult; + + if (!CheckForSerializableConflictOutNeeded(relation, snapshot)) + return; + + /* + * Check to see whether the tuple has been written to by a concurrent + * transaction, either to create it not visible to us, or to delete it + * 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. + */ + htsvResult = HeapTupleSatisfiesVacuum(tuple, TransactionXmin, buffer); + switch (htsvResult) + { + case HEAPTUPLE_LIVE: + if (visible) + return; + 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); + break; + case HEAPTUPLE_INSERT_IN_PROGRESS: + xid = HeapTupleHeaderGetXmin(tuple->t_data); + break; + case HEAPTUPLE_DEAD: + return; + default: + + /* + * The only way to get to this default clause is if a new value is + * added to the enum type without adding it to this switch + * statement. That's a bug, so elog. + */ + elog(ERROR, "unrecognized return value from HeapTupleSatisfiesVacuum: %u", htsvResult); + + /* + * In spite of having all enum values covered and calling elog on + * this default, some compilers think this is a code path which + * allows xid to be used below without initialization. Silence + * that warning. + */ + xid = InvalidTransactionId; + } + + return CheckForSerializableConflictOut(relation, xid, true, snapshot); +} diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 09bc6fe98a7..bad26bb934b 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2164,9 +2164,10 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, if (valid) { hscan->rs_vistuples[ntup++] = offnum; - PredicateLockTuple(scan->rs_rd, &loctup, snapshot); + PredicateLockTID(scan->rs_rd, &loctup.t_self, snapshot, + HeapTupleHeaderGetXmin(loctup.t_data)); } - CheckForSerializableConflictOut(valid, scan->rs_rd, &loctup, + HeapCheckForSerializableConflictOut(valid, scan->rs_rd, &loctup, buffer, snapshot); } } @@ -2354,7 +2355,7 @@ heapam_scan_sample_next_tuple(TableScanDesc scan, SampleScanState *scanstate, /* in pagemode, heapgetpage did this for us */ if (!pagemode) - CheckForSerializableConflictOut(visible, scan->rs_rd, tuple, + HeapCheckForSerializableConflictOut(visible, scan->rs_rd, tuple, hscan->rs_cbuf, scan->rs_snapshot); /* Try next tuple from same page. */ diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c index 28edd4aca76..6f1093ae383 100644 --- a/src/backend/access/index/indexam.c +++ b/src/backend/access/index/indexam.c @@ -180,8 +180,8 @@ index_insert(Relation indexRelation, if (!(indexRelation->rd_indam->ampredlocks)) CheckForSerializableConflictIn(indexRelation, - (HeapTuple) NULL, - InvalidBuffer); + (ItemPointer) NULL, + InvalidBlockNumber); return indexRelation->rd_indam->aminsert(indexRelation, values, isnull, heap_t_ctid, heapRelation, diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c index 602f8849d4a..22fcb26a069 100644 --- a/src/backend/access/nbtree/nbtinsert.c +++ b/src/backend/access/nbtree/nbtinsert.c @@ -290,7 +290,7 @@ top: * checkingunique and !heapkeyspace cases, but it's okay to use the * first page the value could be on (with scantid omitted) instead. */ - CheckForSerializableConflictIn(rel, NULL, insertstate.buf); + CheckForSerializableConflictIn(rel, NULL, BufferGetBlockNumber(insertstate.buf)); /* * Do the insertion. Note that insertstate contains cached binary @@ -533,7 +533,7 @@ _bt_check_unique(Relation rel, BTInsertState insertstate, Relation heapRel, * otherwise be masked by this unique constraint * violation. */ - CheckForSerializableConflictIn(rel, NULL, insertstate->buf); + CheckForSerializableConflictIn(rel, NULL, BufferGetBlockNumber(insertstate->buf)); /* * This is a definite conflict. Break the tuple down into diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index 2d709420c3d..b4e5b893e4f 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -163,8 +163,8 @@ * PredicateLockRelation(Relation relation, Snapshot snapshot) * PredicateLockPage(Relation relation, BlockNumber blkno, * Snapshot snapshot) - * PredicateLockTuple(Relation relation, HeapTuple tuple, - * Snapshot snapshot) + * PredicateLockTID(Relation relation, ItemPointer tid, Snapshot snapshot, + * TransactionId insert_xid) * PredicateLockPageSplit(Relation relation, BlockNumber oldblkno, * BlockNumber newblkno) * PredicateLockPageCombine(Relation relation, BlockNumber oldblkno, @@ -173,11 +173,10 @@ * ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe) * * conflict detection (may also trigger rollback) - * CheckForSerializableConflictOut(bool visible, Relation relation, - * HeapTupleData *tup, Buffer buffer, + * CheckForSerializableConflictOut(Relation relation, TransactionId xid, * Snapshot snapshot) - * CheckForSerializableConflictIn(Relation relation, HeapTupleData *tup, - * Buffer buffer) + * CheckForSerializableConflictIn(Relation relation, ItemPointer tid, + * BlockNumber blkno) * CheckTableForSerializableConflictIn(Relation relation) * * final rollback checking @@ -193,8 +192,6 @@ #include "postgres.h" -#include "access/heapam.h" -#include "access/htup_details.h" #include "access/parallel.h" #include "access/slru.h" #include "access/subtrans.h" @@ -2538,37 +2535,34 @@ PredicateLockPage(Relation relation, BlockNumber blkno, Snapshot snapshot) } /* - * PredicateLockTuple + * PredicateLockTID * * Gets a predicate lock at the tuple level. * Skip if not in full serializable transaction isolation level. * Skip if this is a temporary table. */ void -PredicateLockTuple(Relation relation, HeapTuple tuple, Snapshot snapshot) +PredicateLockTID(Relation relation, ItemPointer tid, Snapshot snapshot, + TransactionId insert_xid) { PREDICATELOCKTARGETTAG tag; - ItemPointer tid; - TransactionId targetxmin; if (!SerializationNeededForRead(relation, snapshot)) return; /* - * If it's a heap tuple, return if this xact wrote it. + * Return if this xact wrote it. */ if (relation->rd_index == NULL) { TransactionId myxid; - targetxmin = HeapTupleHeaderGetXmin(tuple->t_data); - myxid = GetTopTransactionIdIfAny(); if (TransactionIdIsValid(myxid)) { - if (TransactionIdFollowsOrEquals(targetxmin, TransactionXmin)) + if (TransactionIdFollowsOrEquals(insert_xid, TransactionXmin)) { - TransactionId xid = SubTransGetTopmostTransaction(targetxmin); + TransactionId xid = SubTransGetTopmostTransaction(insert_xid); if (TransactionIdEquals(xid, myxid)) { @@ -2591,7 +2585,6 @@ PredicateLockTuple(Relation relation, HeapTuple tuple, Snapshot snapshot) if (PredicateLockExists(&tag)) return; - tid = &(tuple->t_self); SET_PREDICATELOCKTARGETTAG_TUPLE(tag, relation->rd_node.dbNode, relation->rd_id, @@ -4036,33 +4029,44 @@ XidIsConcurrent(TransactionId xid) return false; } +bool +CheckForSerializableConflictOutNeeded(Relation relation, Snapshot snapshot) +{ + if (!SerializationNeededForRead(relation, snapshot)) + return false; + + /* Check if someone else has already decided that we need to die */ + if (SxactIsDoomed(MySerializableXact)) + { + ereport(ERROR, + (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), + errmsg("could not serialize access due to read/write dependencies among transactions"), + errdetail_internal("Reason code: Canceled on identification as a pivot, during conflict out checking."), + errhint("The transaction might succeed if retried."))); + } + + return true; +} + /* * CheckForSerializableConflictOut * 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, + * serializable transaction, that is also a rw-conflict out. * * 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. - * - * 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 - * buffer, because this function might set hint bits on the tuple. There is - * currently no known reason to call this function from an index AM. */ void -CheckForSerializableConflictOut(bool visible, Relation relation, - HeapTuple tuple, Buffer buffer, - Snapshot snapshot) +CheckForSerializableConflictOut(Relation relation, TransactionId xid, + bool needSubtransHandling, Snapshot snapshot) { - TransactionId xid; SERIALIZABLEXIDTAG sxidtag; SERIALIZABLEXID *sxid; SERIALIZABLEXACT *sxact; - HTSV_Result htsvResult; if (!SerializationNeededForRead(relation, snapshot)) return; @@ -4077,51 +4081,6 @@ CheckForSerializableConflictOut(bool visible, Relation relation, errhint("The transaction might succeed if retried."))); } - /* - * Check to see whether the tuple has been written to by a concurrent - * transaction, either to create it not visible to us, or to delete it - * 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. - */ - htsvResult = HeapTupleSatisfiesVacuum(tuple, TransactionXmin, buffer); - switch (htsvResult) - { - case HEAPTUPLE_LIVE: - if (visible) - return; - 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); - break; - case HEAPTUPLE_INSERT_IN_PROGRESS: - xid = HeapTupleHeaderGetXmin(tuple->t_data); - break; - case HEAPTUPLE_DEAD: - return; - default: - - /* - * The only way to get to this default clause is if a new value is - * added to the enum type without adding it to this switch - * statement. That's a bug, so elog. - */ - elog(ERROR, "unrecognized return value from HeapTupleSatisfiesVacuum: %u", htsvResult); - - /* - * In spite of having all enum values covered and calling elog on - * this default, some compilers think this is a code path which - * allows xid to be used below without initialization. Silence - * that warning. - */ - xid = InvalidTransactionId; - } Assert(TransactionIdIsValid(xid)); Assert(TransactionIdFollowsOrEquals(xid, TransactionXmin)); @@ -4131,11 +4090,15 @@ CheckForSerializableConflictOut(bool visible, Relation relation, */ if (TransactionIdEquals(xid, GetTopTransactionIdIfAny())) return; - xid = SubTransGetTopmostTransaction(xid); - if (TransactionIdPrecedes(xid, TransactionXmin)) - return; - if (TransactionIdEquals(xid, GetTopTransactionIdIfAny())) - return; + + if (needSubtransHandling) + { + xid = SubTransGetTopmostTransaction(xid); + if (TransactionIdPrecedes(xid, TransactionXmin)) + return; + if (TransactionIdEquals(xid, GetTopTransactionIdIfAny())) + return; + } /* * Find sxact or summarized info for the top level xid. @@ -4439,8 +4402,7 @@ CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag) * tuple itself. */ void -CheckForSerializableConflictIn(Relation relation, HeapTuple tuple, - Buffer buffer) +CheckForSerializableConflictIn(Relation relation, ItemPointer tid, BlockNumber blkno) { PREDICATELOCKTARGETTAG targettag; @@ -4470,22 +4432,22 @@ CheckForSerializableConflictIn(Relation relation, HeapTuple tuple, * It is not possible to take and hold a lock across the checks for all * granularities because each target could be in a separate partition. */ - if (tuple != NULL) + if (tid != NULL) { SET_PREDICATELOCKTARGETTAG_TUPLE(targettag, relation->rd_node.dbNode, relation->rd_id, - ItemPointerGetBlockNumber(&(tuple->t_self)), - ItemPointerGetOffsetNumber(&(tuple->t_self))); + ItemPointerGetBlockNumber(tid), + ItemPointerGetOffsetNumber(tid)); CheckTargetForConflictsIn(&targettag); } - if (BufferIsValid(buffer)) + if (blkno != InvalidBlockNumber) { SET_PREDICATELOCKTARGETTAG_PAGE(targettag, relation->rd_node.dbNode, relation->rd_id, - BufferGetBlockNumber(buffer)); + blkno); CheckTargetForConflictsIn(&targettag); } diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 858bcb6bc96..390023c0a55 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -217,5 +217,7 @@ extern bool ResolveCminCmaxDuringDecoding(struct HTAB *tuplecid_data, HeapTuple htup, Buffer buffer, CommandId *cmin, CommandId *cmax); +extern void HeapCheckForSerializableConflictOut(bool valid, Relation relation, HeapTuple tuple, + Buffer buffer, Snapshot snapshot); #endif /* HEAPAM_H */ diff --git a/src/include/storage/predicate.h b/src/include/storage/predicate.h index 376245ecd70..f5fe16e1815 100644 --- a/src/include/storage/predicate.h +++ b/src/include/storage/predicate.h @@ -57,16 +57,18 @@ extern void SetSerializableTransactionSnapshot(Snapshot snapshot, extern void RegisterPredicateLockingXid(TransactionId xid); extern void PredicateLockRelation(Relation relation, Snapshot snapshot); extern void PredicateLockPage(Relation relation, BlockNumber blkno, Snapshot snapshot); -extern void PredicateLockTuple(Relation relation, HeapTuple tuple, Snapshot snapshot); +extern void PredicateLockTID(Relation relation, ItemPointer tid, Snapshot snapshot, + TransactionId insert_xid); extern void PredicateLockPageSplit(Relation relation, BlockNumber oldblkno, BlockNumber newblkno); extern void PredicateLockPageCombine(Relation relation, BlockNumber oldblkno, BlockNumber newblkno); extern void TransferPredicateLocksToHeapRelation(Relation relation); extern void ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe); /* conflict detection (may also trigger rollback) */ -extern void CheckForSerializableConflictOut(bool valid, Relation relation, HeapTuple tuple, - Buffer buffer, Snapshot snapshot); -extern void CheckForSerializableConflictIn(Relation relation, HeapTuple tuple, Buffer buffer); +extern bool CheckForSerializableConflictOutNeeded(Relation relation, Snapshot snapshot); +extern void CheckForSerializableConflictOut(Relation relation, TransactionId xid, + bool needSubtransHandling, Snapshot snapshot); +extern void CheckForSerializableConflictIn(Relation relation, ItemPointer tid, BlockNumber blkno); extern void CheckTableForSerializableConflictIn(Relation relation); /* final rollback checking */ -- 2.19.1