From 14c57cb63907eb7af0f973022b919c0f777db0d9 Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Tue, 24 Sep 2024 15:25:18 -0700 Subject: For inplace update durability, make heap_update() callers wait. The previous commit fixed some ways of losing an inplace update. It remained possible to lose one when a backend working toward a heap_update() copied a tuple into memory just before inplace update of that tuple. In catalogs eligible for inplace update, use LOCKTAG_TUPLE to govern admission to the steps of copying an old tuple, modifying it, and issuing heap_update(). This includes MERGE commands. To avoid changing most of the pg_class DDL, don't require LOCKTAG_TUPLE when holding a relation lock sufficient to exclude inplace updaters. Back-patch to v12 (all supported versions). In v13 and v12, "UPDATE pg_class" or "UPDATE pg_database" can still lose an inplace update. The v14+ UPDATE fix needs commit 86dc90056dfdbd9d1b891718d2e5614e3e432f35, and it wasn't worth reimplementing that fix without such infrastructure. Reviewed by Nitin Motiani and (in earlier versions) Heikki Linnakangas. Discussion: https://postgr.es/m/20231027214946.79.nmisch@google.com --- src/backend/executor/nodeModifyTable.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'src/backend/executor/nodeModifyTable.c') diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 1242c1da3f5..d25074181f9 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1282,6 +1282,7 @@ ExecUpdate(ModifyTableState *mtstate, } else { + ItemPointerData lockedtid PG_USED_FOR_ASSERTS_ONLY; LockTupleMode lockmode; bool partition_constraint_failed; bool update_indexes; @@ -1472,6 +1473,26 @@ lreplace: if (resultRelationDesc->rd_att->constr) ExecConstraints(resultRelInfo, slot, estate); + /* + * We lack the infrastructure to follow rules in README.tuplock + * section "Locking to write inplace-updated tables". Specifically, + * we lack infrastructure to lock tupleid before this file's + * ExecProcNode() call fetches the tuple's old columns. Just take a + * lock that silences check_lock_if_inplace_updateable_rel(). This + * doesn't actually protect inplace updates like those rules intend, + * so we may lose an inplace update that overlaps a superuser running + * "UPDATE pg_class" or "UPDATE pg_database". + */ +#ifdef USE_ASSERT_CHECKING + if (IsInplaceUpdateRelation(resultRelationDesc)) + { + lockedtid = *tupleid; + LockTuple(resultRelationDesc, &lockedtid, InplaceUpdateTupleLock); + } + else + ItemPointerSetInvalid(&lockedtid); +#endif + /* * replace the heap tuple * @@ -1488,6 +1509,11 @@ lreplace: true /* wait for commit */ , &tmfd, &lockmode, &update_indexes); +#ifdef USE_ASSERT_CHECKING + if (ItemPointerIsValid(&lockedtid)) + UnlockTuple(resultRelationDesc, &lockedtid, InplaceUpdateTupleLock); +#endif + switch (result) { case TM_SelfModified: -- cgit v1.2.3