database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-11-03 09:13:20 +03:00
Code Activity

Revert "For inplace update, send nontransactional invalidations."

Browse Source 
This reverts commit 95c5acb3fc (v17) and
counterparts in each other non-master branch.  If released, that commit
would have caused a worst-in-years minor release regression, via
undetected LWLock self-deadlock.  This commit and its self-deadlock fix
warrant more bake time in the master branch.

Reported by Alexander Lakhin.

Discussion: https://postgr.es/m/10ec0bc3-5933-1189-6bb8-5dec4114558e@gmail.com
This commit is contained in:
Noah Misch
2024-11-02 09:05:00 -07:00
parent e50f9de98d
commit 27642d8908
12 changed files with 132 additions and 320 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
src/backend/access/heap/heapam.c
View File

@@ -6306,24 +6306,6 @@ heap_inplace_update_and_unlock(Relation relation,
if (oldlen != newlen || htup->t_hoff != tuple->t_data->t_hoff)
elog(ERROR, "wrong tuple length");
/*
* Construct shared cache inval if necessary. Note that because we only
* pass the new version of the tuple, this mustn't be used for any
* operations that could change catcache lookup keys. But we aren't
* bothering with index updates either, so that's true a fortiori.
*/
CacheInvalidateHeapTupleInplace(relation, tuple, NULL);
/*
* Unlink relcache init files as needed. If unlinking, acquire
* RelCacheInitLock until after associated invalidations. By doing this
* in advance, if we checkpoint and then crash between inplace
* XLogInsert() and inval, we don't rely on StartupXLOG() ->
* RelationCacheInitFileRemove(). That uses elevel==LOG, so replay would
* neglect to PANIC on EIO.
*/
PreInplace_Inval();
/* NO EREPORT(ERROR) from here till changes are logged */
START_CRIT_SECTION();
@@ -6367,28 +6349,17 @@ heap_inplace_update_and_unlock(Relation relation,
PageSetLSN(BufferGetPage(buffer), recptr);
}
LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
/*
* Send invalidations to shared queue. SearchSysCacheLocked1() assumes we
* do this before UnlockTuple().
*
* If we're mutating a tuple visible only to this transaction, there's an
* equivalent transactional inval from the action that created the tuple,
* and this inval is superfluous.
*/
AtInplace_Inval();
END_CRIT_SECTION();
UnlockTuple(relation, &tuple->t_self, InplaceUpdateTupleLock);
AcceptInvalidationMessages(); /* local processing of just-sent inval */
heap_inplace_unlock(relation, oldtup, buffer);
/*
* Queue a transactional inval. The immediate invalidation we just sent
* is the only one known to be necessary. To reduce risk from the
* transition to immediate invalidation, continue sending a transactional
* invalidation like we've long done. Third-party code might rely on it.
* Send out shared cache inval if necessary. Note that because we only
* pass the new version of the tuple, this mustn't be used for any
* operations that could change catcache lookup keys. But we aren't
* bothering with index updates either, so that's true a fortiori.
*
* XXX ROLLBACK discards the invalidation. See test inplace-inval.spec.
*/
if (!IsBootstrapProcessingMode())
CacheInvalidateHeapTuple(relation, tuple, NULL);

26
src/backend/access/transam/xact.c
View File

@@ -1333,24 +1333,14 @@ RecordTransactionCommit(void)
/*
* Transactions without an assigned xid can contain invalidation
* messages. While inplace updates do this, this is not known to be
* necessary; see comment at inplace CacheInvalidateHeapTuple().
* Extensions might still rely on this capability, and standbys may
* need to process those invals. We can't emit a commit record
* without an xid, and we don't want to force assigning an xid,
* because that'd be problematic for e.g. vacuum. Hence we emit a
* bespoke record for the invalidations. We don't want to use that in
* case a commit record is emitted, so they happen synchronously with
* commits (besides not wanting to emit more WAL records).
*
* XXX Every known use of this capability is a defect. Since an XID
* isn't controlling visibility of the change that prompted invals,
* other sessions need the inval even if this transactions aborts.
*
* ON COMMIT DELETE ROWS does a nontransactional index_build(), which
* queues a relcache inval, including in transactions without an xid
* that had read the (empty) table. Standbys don't need any ON COMMIT
* DELETE ROWS invals, but we've not done the work to withhold them.
* messages (e.g. explicit relcache invalidations or catcache
* invalidations for inplace updates); standbys need to process those.
* We can't emit a commit record without an xid, and we don't want to
* force assigning an xid, because that'd be problematic for e.g.
* vacuum. Hence we emit a bespoke record for the invalidations. We
* don't want to use that in case a commit record is emitted, so they
* happen synchronously with commits (besides not wanting to emit more
* WAL records).
*/
if (nmsgs != 0)
{