Andres Freund wrote: > I have to say, it makes me really uncomfortable to take such > shortcuts. In other branches we are doing liveliness checks with > MultiXactIdIsRunning() et al. Why isn't that necessary here? And how > likely is that this won't cause breakage somewhere down the line because > somebody doesn't know of that subtlety?
I came up with the attached last night, which should do the right thing in both cases. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
*** a/src/backend/access/transam/multixact.c --- b/src/backend/access/transam/multixact.c *************** *** 1275,1280 **** retry: --- 1275,1313 ---- } /* + * MultiXactHasRunningRemoteMembers + * Does the given multixact have still-live members from + * transactions other than our own? + */ + bool + MultiXactHasRunningRemoteMembers(MultiXactId multi) + { + MultiXactMember *members; + int nmembers; + int i; + + nmembers = GetMultiXactIdMembers(multi, &members, true); + if (nmembers <= 0) + return false; + + for (i = 0; i < nmembers; i++) + { + /* not interested in our own members */ + if (TransactionIdIsCurrentTransactionId(members[i].xid)) + continue; + + if (TransactionIdIsInProgress(members[i].xid)) + { + pfree(members); + return true; + } + } + + pfree(members); + return false; + } + + /* * mxactMemberComparator * qsort comparison function for MultiXactMember * *** a/src/backend/utils/time/tqual.c --- b/src/backend/utils/time/tqual.c *************** *** 686,693 **** HeapTupleSatisfiesUpdate(HeapTupleHeader tuple, CommandId curcid, if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid */ return HeapTupleMayBeUpdated; ! if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)) /* not deleter */ ! return HeapTupleMayBeUpdated; if (tuple->t_infomask & HEAP_XMAX_IS_MULTI) { --- 686,721 ---- if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid */ return HeapTupleMayBeUpdated; ! if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)) ! { ! TransactionId xmax; ! ! xmax = HeapTupleHeaderGetRawXmax(tuple); ! ! /* ! * Careful here: even though this tuple was created by our own ! * transaction, it might be locked by other transactions, if ! * the original version was key-share locked when we updated ! * it. ! */ ! ! if (tuple->t_infomask & HEAP_XMAX_IS_MULTI) ! { ! if (MultiXactHasRunningRemoteMembers(xmax)) ! return HeapTupleBeingUpdated; ! else ! return HeapTupleMayBeUpdated; ! } ! ! /* if locker is gone, all's well */ ! if (!TransactionIdIsInProgress(xmax)) ! return HeapTupleMayBeUpdated; ! ! if (!TransactionIdIsCurrentTransactionId(xmax)) ! return HeapTupleBeingUpdated; ! else ! return HeapTupleMayBeUpdated; ! } if (tuple->t_infomask & HEAP_XMAX_IS_MULTI) { *************** *** 700,706 **** HeapTupleSatisfiesUpdate(HeapTupleHeader tuple, CommandId curcid, --- 728,738 ---- /* updating subtransaction must have aborted */ if (!TransactionIdIsCurrentTransactionId(xmax)) + { + if (MultiXactHasRunningRemoteMembers(xmax)) + return HeapTupleBeingUpdated; return HeapTupleMayBeUpdated; + } else { if (HeapTupleHeaderGetCmax(tuple) >= curcid) *** a/src/include/access/multixact.h --- b/src/include/access/multixact.h *************** *** 89,94 **** extern bool MultiXactIdIsRunning(MultiXactId multi); --- 89,95 ---- extern void MultiXactIdSetOldestMember(void); extern int GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **xids, bool allow_old); + extern bool MultiXactHasRunningRemoteMembers(MultiXactId multi); extern bool MultiXactIdPrecedes(MultiXactId multi1, MultiXactId multi2); extern bool MultiXactIdPrecedesOrEquals(MultiXactId multi1, MultiXactId multi2);
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers