Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitce8c571

Browse files
committed
For inplace update, send nontransactional invalidations.
The inplace update survives ROLLBACK. The inval didn't, so anotherbackend's DDL could then update the row without incorporating theinplace update. In the test this fixes, a mix of CREATE INDEX and ALTERTABLE resulted in a table with an index, yet relhasindex=f. That is asource of index corruption. Back-patch to v12 (all supported versions).The back branch versions don't change WAL, because those branches justadded end-of-recovery SIResetAll(). All branches change the ABI ofextern function PrepareToInvalidateCacheTuple(). No PGXN extensioncalls that, and there's no apparent use case in extensions.Reviewed by Nitin Motiani and (in earlier versions) Andres Freund.Discussion:https://postgr.es/m/20240523000548.58.nmisch@google.com
1 parentd36b4d8 commitce8c571

File tree

12 files changed

+320
-132
lines changed

12 files changed

+320
-132
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6191,6 +6191,24 @@ heap_inplace_update_and_unlock(Relation relation,
61916191
if (oldlen!=newlen||htup->t_hoff!=tuple->t_data->t_hoff)
61926192
elog(ERROR,"wrong tuple length");
61936193

6194+
/*
6195+
* Construct shared cache inval if necessary. Note that because we only
6196+
* pass the new version of the tuple, this mustn't be used for any
6197+
* operations that could change catcache lookup keys. But we aren't
6198+
* bothering with index updates either, so that's true a fortiori.
6199+
*/
6200+
CacheInvalidateHeapTupleInplace(relation,tuple,NULL);
6201+
6202+
/*
6203+
* Unlink relcache init files as needed. If unlinking, acquire
6204+
* RelCacheInitLock until after associated invalidations. By doing this
6205+
* in advance, if we checkpoint and then crash between inplace
6206+
* XLogInsert() and inval, we don't rely on StartupXLOG() ->
6207+
* RelationCacheInitFileRemove(). That uses elevel==LOG, so replay would
6208+
* neglect to PANIC on EIO.
6209+
*/
6210+
PreInplace_Inval();
6211+
61946212
/* NO EREPORT(ERROR) from here till changes are logged */
61956213
START_CRIT_SECTION();
61966214

@@ -6234,17 +6252,28 @@ heap_inplace_update_and_unlock(Relation relation,
62346252
PageSetLSN(BufferGetPage(buffer),recptr);
62356253
}
62366254

6255+
LockBuffer(buffer,BUFFER_LOCK_UNLOCK);
6256+
6257+
/*
6258+
* Send invalidations to shared queue. SearchSysCacheLocked1() assumes we
6259+
* do this before UnlockTuple().
6260+
*
6261+
* If we're mutating a tuple visible only to this transaction, there's an
6262+
* equivalent transactional inval from the action that created the tuple,
6263+
* and this inval is superfluous.
6264+
*/
6265+
AtInplace_Inval();
6266+
62376267
END_CRIT_SECTION();
6268+
UnlockTuple(relation,&tuple->t_self,InplaceUpdateTupleLock);
62386269

6239-
heap_inplace_unlock(relation,oldtup,buffer);
6270+
AcceptInvalidationMessages();/* local processing of just-sent inval */
62406271

62416272
/*
6242-
* Send out shared cache inval if necessary. Note that because we only
6243-
* pass the new version of the tuple, this mustn't be used for any
6244-
* operations that could change catcache lookup keys. But we aren't
6245-
* bothering with index updates either, so that's true a fortiori.
6246-
*
6247-
* XXX ROLLBACK discards the invalidation. See test inplace-inval.spec.
6273+
* Queue a transactional inval. The immediate invalidation we just sent
6274+
* is the only one known to be necessary. To reduce risk from the
6275+
* transition to immediate invalidation, continue sending a transactional
6276+
* invalidation like we've long done. Third-party code might rely on it.
62486277
*/
62496278
if (!IsBootstrapProcessingMode())
62506279
CacheInvalidateHeapTuple(relation,tuple,NULL);

‎src/backend/access/transam/xact.c

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,14 +1337,24 @@ RecordTransactionCommit(void)
13371337

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

‎src/backend/catalog/index.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2915,12 +2915,19 @@ index_update_stats(Relation rel,
29152915
if (dirty)
29162916
{
29172917
systable_inplace_update_finish(state,tuple);
2918-
/* the above sendsacache invalmessage */
2918+
/* the above sendstransactional and immediatecache invalmessages */
29192919
}
29202920
else
29212921
{
29222922
systable_inplace_update_cancel(state);
2923-
/* no need to change tuple, but force relcache inval anyway */
2923+
2924+
/*
2925+
* While we didn't change relhasindex, CREATE INDEX needs a
2926+
* transactional inval for when the new index's catalog rows become
2927+
* visible. Other CREATE INDEX and REINDEX code happens to also queue
2928+
* this inval, but keep this in case rare callers rely on this part of
2929+
* our API contract.
2930+
*/
29242931
CacheInvalidateRelcacheByTuple(tuple);
29252932
}
29262933

‎src/backend/replication/logical/decode.c

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -511,23 +511,19 @@ heap_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
511511

512512
/*
513513
* Inplace updates are only ever performed on catalog tuples and
514-
* can, per definition, not change tuple visibility. Since we
515-
* don't decode catalog tuples, we're not interested in the
516-
* record's contents.
514+
* can, per definition, not change tuple visibility. Inplace
515+
* updates don't affect storage or interpretation of table rows,
516+
* so they don't affect logicalrep_write_tuple() outcomes. Hence,
517+
* we don't process invalidations from the original operation. If
518+
* inplace updates did affect those things, invalidations wouldn't
519+
* make it work, since there are no snapshot-specific versions of
520+
* inplace-updated values. Since we also don't decode catalog
521+
* tuples, we're not interested in the record's contents.
517522
*
518-
* In-place updates can be used either by XID-bearing transactions
519-
* (e.g. in CREATE INDEX CONCURRENTLY) or by XID-less
520-
* transactions (e.g. VACUUM). In the former case, the commit
521-
* record will include cache invalidations, so we mark the
522-
* transaction as catalog modifying here. Currently that's
523-
* redundant because the commit will do that as well, but once we
524-
* support decoding in-progress relations, this will be important.
523+
* WAL contains likely-unnecessary commit-time invals from the
524+
* CacheInvalidateHeapTuple() call in heap_inplace_update().
525+
* Excess invalidation is safe.
525526
*/
526-
if (!TransactionIdIsValid(xid))
527-
break;
528-
529-
(void)SnapBuildProcessChange(builder,xid,buf->origptr);
530-
ReorderBufferXidSetCatalogChanges(ctx->reorder,xid,buf->origptr);
531527
break;
532528

533529
caseXLOG_HEAP_CONFIRM:

‎src/backend/utils/cache/catcache.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2219,7 +2219,8 @@ void
22192219
PrepareToInvalidateCacheTuple(Relationrelation,
22202220
HeapTupletuple,
22212221
HeapTuplenewtuple,
2222-
void (*function) (int,uint32,Oid))
2222+
void (*function) (int,uint32,Oid,void*),
2223+
void*context)
22232224
{
22242225
slist_iteriter;
22252226
Oidreloid;
@@ -2260,7 +2261,7 @@ PrepareToInvalidateCacheTuple(Relation relation,
22602261
hashvalue=CatalogCacheComputeTupleHashValue(ccp,ccp->cc_nkeys,tuple);
22612262
dbid=ccp->cc_relisshared ? (Oid)0 :MyDatabaseId;
22622263

2263-
(*function) (ccp->id,hashvalue,dbid);
2264+
(*function) (ccp->id,hashvalue,dbid,context);
22642265

22652266
if (newtuple)
22662267
{
@@ -2269,7 +2270,7 @@ PrepareToInvalidateCacheTuple(Relation relation,
22692270
newhashvalue=CatalogCacheComputeTupleHashValue(ccp,ccp->cc_nkeys,newtuple);
22702271

22712272
if (newhashvalue!=hashvalue)
2272-
(*function) (ccp->id,newhashvalue,dbid);
2273+
(*function) (ccp->id,newhashvalue,dbid,context);
22732274
}
22742275
}
22752276
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp