[PATCH v1 0/1] Avoid carrying self lock-only xmax to updated tuple

Hi,

heap_update() normally carries pre-existing tuple lockers from the old
tuple version to the new one. That behavior is required when the old
tuple is locked by another transaction, in particular for FOR KEY SHARE
locks taken by referential-integrity checks. Such remote lockers must
continue to protect the updated row version.

However, when the old tuple's xmax is a single-XID, lock-only xmax, and
that XID is the same XID that is now updating the tuple, there is no
independent locker to carry forward. In the single-XID case,
heap_update() carries the old xmax to the new tuple only as
HEAP_XMAX_KEYSHR_LOCK | HEAP_XMAX_LOCK_ONLY, with no HEAP_KEYS_UPDATED
bit. That is useful for preserving a remote key-share locker, but it is
redundant for a self-owned lock: the new tuple's xmin is the updating
XID, so other transactions cannot treat the new tuple as a visible
committed tuple while the update is in progress, and the self-owned lock
ends when that same transaction ends.

One simple way to hit this repeatedly is a PL/pgSQL loop with an
exception block, since each block is run in a subtransaction:

CREATE TABLE lock_carry_counter (
tenant_id text NOT NULL,
counter_name text NOT NULL,
counter_value bigint,
PRIMARY KEY (tenant_id, counter_name)
);

INSERT INTO lock_carry_counter VALUES ('t1', 'main', 1);

CREATE FUNCTION lock_carry_counter_bump(n integer) RETURNS void
LANGUAGE plpgsql AS $$
DECLARE
current_value bigint;
BEGIN
FOR i IN 1..n LOOP
BEGIN
SELECT counter_value INTO current_value
FROM lock_carry_counter
WHERE tenant_id = 't1' AND counter_name = 'main'
FOR UPDATE;

UPDATE lock_carry_counter
SET counter_value = counter_value + 1
WHERE tenant_id = 't1' AND counter_name = 'main';
EXCEPTION WHEN OTHERS THEN
NULL;
END;
END LOOP;
END;
$$;

Before the patch, each new tuple version can carry the previous
subtransaction's self-owned lock-only xmax. Later iterations then have
to process that carried xmax, and can end up doing unnecessary MultiXact
work. After the patch, the chain tail has an invalid xmax in this
same-XID case.

In a local benchmark of the repeated subtransaction pattern above, with
fsync disabled, 10000 iterations took about 126s before the patch and
about 27s after the patch, roughly a 4x improvement. The speedup comes
from avoiding the repeated creation/expansion and rereading of MultiXacts
caused by carrying the previous subtransaction's self lock-only xmax to
the new tuple version. This is not intended as a general UPDATE
performance claim; it describes this specific repeated lock/update
pattern.

The patch changes heap_update() so that, after the MultiXact case has
already been excluded, a single-XID lock-only xmax equal to the current
update XID is treated as having no remaining locker to carry to the new
tuple. Remote lockers, different subtransactions, and MultiXacts keep
the existing behavior.

The included pageinspect regression test and README.tuplock update are
intended to make the behavior and invariant easier to review: the test
exposes that the chain tail no longer carries a self lock-only xmax or
creates a MultiXact, and the README notes that single-XID carry-forward
only preserves a KEYSHR_LOCK | LOCK_ONLY marker, which is needed for
remote key-share lockers but redundant for a same-XID self locker.
Feedback on whether to keep, move, or drop either part would be welcome.

This issue was previously discussed in 2019:

https://postgr.es/m/20190724232439.lpxzjw2jg3ukgcqn@alap3.anarazel.de

The patch addresses the single-XID self-lock carry-forward case described
there: ON CONFLICT DO UPDATE and manual row locking can leave the updated
tuple with a redundant self-owned lock-only xmax.

The attached v1 patch is based on master at 0b776de09ed.

Feedback on the correctness of the same-XID exception would be
appreciated.

Regards,
chee.wooson

chee.wooson (1):
Avoid carrying self lock-only xmax to updated tuple

contrib/pageinspect/expected/page.out | 62 ++++++++++++++++++++++++++
contrib/pageinspect/sql/page.sql | 57 +++++++++++++++++++++++
src/backend/access/heap/README.tuplock | 38 ++++++++++++++++
src/backend/access/heap/heapam.c | 13 ++++++
4 files changed, 170 insertions(+)

--
2.43.0