>From 4c7fda01078c368478fbc1ffa916e689cc61ef91 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Sun, 1 Dec 2013 12:32:59 +0100 Subject: [PATCH] Avoid MultiXacts in more situations when updating a previously locked row. Since the introduction of the foreign key locking feature, when updating a row previously locked in the same transaction, we generated a MultiXact when the new lock was weaker than the old one. This could easily happen in applications using explicit FOR UPDATE clauses before UPDATEing, since an UPDATE now often only uses the NO KEY UPDATE lock level. To do so, simply promote the new lock to the old, stronger, lock level when computing the new infomask for the updated tuple. --- src/backend/access/heap/heapam.c | 42 +++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index c13f87c..c03ff80 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -4725,33 +4725,35 @@ l5: new_status = get_mxact_status_for_lock(mode, is_update); /* - * 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. + * Already locked by the current transaction, so we can take a + * shortcut: If the existing lock mode is identical to or weaker than + * the new one, we can act as though there is no existing lock. If the + * old lock mode is stronger than the new one, we just promote the new + * lock mode. To do so, set XMAX_INVALID 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. + * The old xmax cannot not be an update, since otherwise this row + * version would have been invisible. */ - 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)); + + /* Promote lockmode if necessary. */ + if (mode < old_mode) + mode = old_mode; + + /* + * 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. 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; } new_xmax = MultiXactIdCreate(xmax, status, add_to_xmax, new_status); GetMultiXactIdHintBits(new_xmax, &new_infomask, &new_infomask2); -- 1.8.3.251.g1462b67