diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 023db6a..2d6f57d 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -4534,6 +4534,8 @@ compute_new_xmax_infomask(TransactionId xmax, uint16 old_infomask, uint16 new_infomask, new_infomask2; + Assert(TransactionIdIsCurrentTransactionId(add_to_xmax)); + l5: new_infomask = 0; new_infomask2 = 0; @@ -4584,6 +4586,9 @@ l5: else if (old_infomask & HEAP_XMAX_IS_MULTI) { MultiXactStatus new_status; + MultiXactMember *members; + int nmembers; + bool keepmulti = false; /* * Currently we don't allow XMAX_COMMITTED to be set for multis, so @@ -4607,37 +4612,70 @@ l5: } /* - * If the XMAX is already a MultiXactId, then we need to expand it to - * include add_to_xmax; but if all the members were lockers and are - * all gone, we can do away with the IS_MULTI bit and just set - * add_to_xmax as the only locker/updater. If all lockers are gone - * and we have an updater that aborted, we can also do without a - * multi. - * - * The cost of doing GetMultiXactIdMembers would be paid by - * MultiXactIdExpand if we weren't to do this, so this check is not - * incurring extra work anyhow. + * Examine the Multixact members. If any of them is the current + * transaction and holds a lock at least as strong as we are + * requesting, then the lock request can be satisfied with the + * existing MultiXactId; there's no need to create a new one. */ - if (!MultiXactIdIsRunning(xmax)) + nmembers = GetMultiXactIdMembers(xmax, &members, true); + if (nmembers > 0) { - if (HEAP_XMAX_IS_LOCKED_ONLY(old_infomask) || - TransactionIdDidAbort(MultiXactIdGetUpdateXid(xmax, - old_infomask))) + int i; + + for (i = 0; i < nmembers; i++) { - /* - * Reset these bits and restart; otherwise fall through to - * create a new multi below. - */ - old_infomask &= ~HEAP_XMAX_IS_MULTI; - old_infomask |= HEAP_XMAX_INVALID; - goto l5; + if (TransactionIdIsCurrentTransactionId(members[i].xid) && + (TUPLOCK_from_mxstatus(members[i].status) >= mode) && + (!is_update || ISUPDATE_from_mxstatus(members[i].status))) + { + keepmulti = true; + break; + } } + pfree(members); } - new_status = get_mxact_status_for_lock(mode, is_update); + if (keepmulti) + { + new_status = get_mxact_status_for_lock(mode, is_update); + new_xmax = xmax; + } + else + { + /* + * If the XMAX is already a MultiXactId, then we need to expand it + * to include add_to_xmax; but if all the members were lockers and + * are all gone, we can do away with the IS_MULTI bit and just set + * add_to_xmax as the only locker/updater. If all lockers are + * gone and we have an updater that aborted, we can also do + * without a multi. + * + * The cost of doing GetMultiXactIdMembers would be paid by + * MultiXactIdExpand if we weren't to do this, so this check is + * not incurring extra work anyhow. + */ + if (!MultiXactIdIsRunning(xmax)) + { + if (HEAP_XMAX_IS_LOCKED_ONLY(old_infomask) || + TransactionIdDidAbort(MultiXactIdGetUpdateXid(xmax, + old_infomask))) + { + /* + * Reset these bits and restart; otherwise fall through to + * create a new multi below. + */ + old_infomask &= ~HEAP_XMAX_IS_MULTI; + old_infomask |= HEAP_XMAX_INVALID; + goto l5; + } + } + + new_status = get_mxact_status_for_lock(mode, is_update); + + new_xmax = MultiXactIdExpand((MultiXactId) xmax, add_to_xmax, + new_status); + } - new_xmax = MultiXactIdExpand((MultiXactId) xmax, add_to_xmax, - new_status); GetMultiXactIdHintBits(new_xmax, &new_infomask, &new_infomask2); } else if (old_infomask & HEAP_XMAX_COMMITTED) @@ -4664,33 +4702,6 @@ l5: new_xmax = MultiXactIdCreate(xmax, status, add_to_xmax, new_status); GetMultiXactIdHintBits(new_xmax, &new_infomask, &new_infomask2); } - else if (xmax == add_to_xmax && TransactionIdIsCurrentTransactionId(xmax)) - - { - LockTupleMode old_mode = TUPLOCK_from_mxstatus(status); - - /* - * If the lock to be acquired is from the same transaction as the - * existing lock, there's an additional optimization: consider only - * the strongest of both locks as the only one present, and restart. - * - * Note that it's not possible for the original tuple to be updated: - * we wouldn't be here because the tuple would have been invisible and - * we wouldn't try to update it. As a subtlety, this code can also - * run when traversing an update chain to lock future versions of a - * tuple. But we wouldn't be here either, because the add_to_xmax - * would be different from the original updater. - */ - Assert(HEAP_XMAX_IS_LOCKED_ONLY(old_infomask)); - - /* acquire the strongest of both */ - if (mode < old_mode) - mode = old_mode; - /* don't need to touch is_update */ - - old_infomask |= HEAP_XMAX_INVALID; - goto l5; - } else if (TransactionIdIsInProgress(xmax)) { /* @@ -4698,23 +4709,22 @@ l5: * create a new MultiXactId that includes both the old locker or * updater and our own TransactionId. */ - MultiXactStatus status; MultiXactStatus new_status; - - Assert(xmax != add_to_xmax); + MultiXactStatus old_status; + LockTupleMode old_mode; if (HEAP_XMAX_IS_LOCKED_ONLY(old_infomask)) { if (HEAP_XMAX_IS_KEYSHR_LOCKED(old_infomask)) - status = MultiXactStatusForKeyShare; + old_status = MultiXactStatusForKeyShare; else if (HEAP_XMAX_IS_SHR_LOCKED(old_infomask)) - status = MultiXactStatusForShare; + old_status = MultiXactStatusForShare; else if (HEAP_XMAX_IS_EXCL_LOCKED(old_infomask)) { if (old_infomask2 & HEAP_KEYS_UPDATED) - status = MultiXactStatusForUpdate; + old_status = MultiXactStatusForUpdate; else - status = MultiXactStatusForNoKeyUpdate; + old_status = MultiXactStatusForNoKeyUpdate; } else { @@ -4734,13 +4744,72 @@ l5: { /* it's an update, but which kind? */ if (old_infomask2 & HEAP_KEYS_UPDATED) - status = MultiXactStatusUpdate; + old_status = MultiXactStatusUpdate; else - status = MultiXactStatusNoKeyUpdate; + old_status = MultiXactStatusNoKeyUpdate; } + old_mode = TUPLOCK_from_mxstatus(old_status); + + /* + * If the lock to be acquired is for the same TransactionId as the + * existing lock, there's an optimization possible: consider only the + * strongest of both locks as the only one present, and restart. + */ + if (xmax == add_to_xmax) + { + /* + * Note that it's not possible for the original tuple to be updated: + * we wouldn't be here because the tuple would have been invisible and + * we wouldn't try to update it. As a subtlety, this code can also + * run when traversing an update chain to lock future versions of a + * tuple. But we wouldn't be here either, because the add_to_xmax + * would be different from the original updater. + */ + Assert(HEAP_XMAX_IS_LOCKED_ONLY(old_infomask)); + + /* acquire the strongest of both */ + if (mode < old_mode) + mode = old_mode; + /* musn't touch is_update */ + + old_infomask |= HEAP_XMAX_INVALID; + goto l5; + } + + /* + * Another possibility is that xmax is not identical to add_to_xmax, + * but that it is an ancestor. We can simply piggy-back on the + * existing lock, without requiring that a new lock be taken. In this + * case, we require the lock mode being requested to be equal or + * weaker than the original; no upgrading of mode is allowed. + * (otherwise, a third party wanting to acquire a lock that conflicts + * with the stronger new lock but not with the old one would not be + * released if the new subtransaction aborts). + * + * To implement this, make it appear that there is no existing lock, + * and re-request what was present. + * + * Since we know add_to_xmax is the current transaction, then xmax is + * an ancestor of add_to_xmax if it is also a current transaction Id. + */ + if (TransactionIdIsCurrentTransactionId(xmax) && old_mode >= mode) + { + /* as above, the tuple cannot possibly be updated */ + Assert(HEAP_XMAX_IS_LOCKED_ONLY(old_infomask)); + + add_to_xmax = xmax; + /* acquire the strongest of both */ + mode = old_mode; + /* musn't touch is_update */ + old_infomask |= HEAP_XMAX_INVALID; + goto l5; + } + + /* otherwise, just fall back to creating a new multixact */ new_status = get_mxact_status_for_lock(mode, is_update); - new_xmax = MultiXactIdCreate(xmax, status, add_to_xmax, new_status); + new_xmax = MultiXactIdCreate(xmax, old_status, + add_to_xmax, new_status); GetMultiXactIdHintBits(new_xmax, &new_infomask, &new_infomask2); } else if (!HEAP_XMAX_IS_LOCKED_ONLY(old_infomask) &&