Optimize updating a row that's locked by same xid

Updating or locking a row that was already locked by the same
transaction under the same Xid caused a MultiXact to be created; but
this is unnecessary, because there's no usefulness in being able to
differentiate two locks by the same transaction.  In particular, if a
transaction executed SELECT FOR UPDATE followed by an UPDATE that didn't
modify columns of the key, we would dutifully represent the resulting
combination as a multixact -- even though a single key-update is
sufficient.

Optimize the case so that only the strongest of both locks/updates is
represented in Xmax.  This can save some Xmax's from becoming
MultiXacts, which can be a significant optimization.

This missed optimization opportunity was spotted by Andres Freund while
investigating a bug reported by Oliver Seemann in message
CANCipfpfzoYnOz5jj=UZ70_R=CwDHv36dqWSpwsi27vpm1z5sA@mail.gmail.com
and also directly as a performance regression reported by Dong Ye in
message
d54b8387.000012d8.00000010@YED-DEVD1.vmware.com
Reportedly, this patch fixes the performance regression.

Since the missing optimization was reported as a significant performance
regression from 9.2, backpatch to 9.3.

Andres Freund, tweaked by Álvaro Herrera
This commit is contained in:
Alvaro Herrera 2013-12-19 16:39:59 -03:00
parent 084e385a2f
commit 13aa624431
1 changed files with 39 additions and 31 deletions

View File

@ -4705,6 +4705,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;
@ -4712,6 +4714,11 @@ l5:
{
/*
* No previous locker; we just insert our own TransactionId.
*
* Note that it's critical that this case be the first one checked,
* because there are several blocks below that come back to this one
* to implement certain optimizations; old_infomask might contain
* other dirty bits in those cases, but we don't really care.
*/
if (is_update)
{
@ -4837,21 +4844,22 @@ l5:
* create a new MultiXactId that includes both the old locker or
* updater and our own TransactionId.
*/
MultiXactStatus status;
MultiXactStatus new_status;
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
{
@ -4871,43 +4879,43 @@ 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;
}
new_status = get_mxact_status_for_lock(mode, is_update);
old_mode = TUPLOCK_from_mxstatus(old_status);
/*
* If the existing lock mode is identical to or weaker than the new
* one, we can act as though there is no existing lock, so set
* XMAX_INVALID and restart.
* 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)
{
LockTupleMode old_mode = TUPLOCK_from_mxstatus(status);
bool old_isupd = ISUPDATE_from_mxstatus(status);
/*
* We can do this if the new LockTupleMode is higher or equal than
* the old one; and if there was previously an update, we need an
* update, but if there wasn't, then we can accept there not being
* one.
* 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.
*/
if ((mode >= old_mode) && (is_update || !old_isupd))
{
/*
* Note that the infomask might contain some other dirty bits.
* However, since the new infomask is reset to zero, we only
* set what's minimally necessary, and that the case that
* checks HEAP_XMAX_INVALID is the very first above, there is
* no need for extra cleanup of the infomask here.
*/
old_infomask |= HEAP_XMAX_INVALID;
goto l5;
}
Assert(HEAP_XMAX_IS_LOCKED_ONLY(old_infomask));
/* acquire the strongest of both */
if (mode < old_mode)
mode = old_mode;
/* mustn't touch is_update */
old_infomask |= HEAP_XMAX_INVALID;
goto l5;
}
new_xmax = MultiXactIdCreate(xmax, status, add_to_xmax, new_status);
/* otherwise, just fall back to creating a new multixact */
new_status = get_mxact_status_for_lock(mode, is_update);
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) &&