On 2019-05-01 10:06:03 -0700, Andres Freund wrote: > I'm not sure this is the right short-term answer. Why isn't it, for now, > sufficient to do what I suggested with RelationSetNewRelfilenode() not > doing the CommandCounterIncrement(), and reindex_index() then doing the > SetReindexProcessing() before a CommandCounterIncrement()? That's like > ~10 line code change, and a few more with comments. > > There is the danger that the current and above approach basically relies > on there not to be any non-inplace updates during reindex. But at the > moment code does take care to use inplace updates > (cf. index_update_stats()). > > It's not clear to me whether the approach of using > RelationSetIndexList() in reindex_index() would be meaningfully more > robust against non-inplace updates during reindex either - ISTM we'd > just as well skip the necessary index insertions if we hid the index > being rebuilt. Skipping to-be-rebuilt indexes works for > reindex_relation() because they're going to be rebuilt subsequently (and > thus the missing index rows don't matter) - but it'd not work for > reindexing a single index, because it'll not get the result at a later > stage.
FWIW, the dirty-hack version (attached) of the CommandCounterIncrement() approach fixes the issue for a REINDEX pg_class_oid_index; in solation even when using CCA. Started a whole CCA testrun with it, but the results of that will obviously not be in quick. Greetings, Andres Freund
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index a05b6a07ad0..6698262e202 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -6108,6 +6108,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, /* Process xmin */ xid = HeapTupleHeaderGetXmin(tuple); + if (xid == FrozenTransactionId) xmin_frozen = ((xid == FrozenTransactionId) || HeapTupleHeaderXminFrozen(tuple)); if (TransactionIdIsNormal(xid)) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index ce024101fc9..98f26e13627 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3344,6 +3344,8 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, /* Suppress use of the target index while rebuilding it */ SetReindexProcessing(heapId, indexId); + CommandCounterIncrement(); + /* Initialize the index and rebuild */ /* Note: we do not need to re-establish pkey setting */ index_build(heapRelation, iRel, indexInfo, true, true); diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index e9add1b9873..f0a4bac75f6 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -315,6 +315,7 @@ ResetSequence(Oid seq_relid) * Create a new storage file for the sequence. */ RelationSetNewRelfilenode(seq_rel, seq_rel->rd_rel->relpersistence); + CommandCounterIncrement(); /* * Ensure sequence's relfrozenxid is at 0, since it won't contain any @@ -490,6 +491,7 @@ AlterSequence(ParseState *pstate, AlterSeqStmt *stmt) * changes transactional. */ RelationSetNewRelfilenode(seqrel, seqrel->rd_rel->relpersistence); + CommandCounterIncrement(); /* * Ensure sequence's relfrozenxid is at 0, since it won't contain any diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f6c9cf3b0ec..eef922d8718 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1791,6 +1791,8 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged, table_close(toastrel, NoLock); } + CommandCounterIncrement(); + /* * Reconstruct the indexes to match, and we're done. */ diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 90ff8ccf54f..2bdfb2d5a9c 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -3541,7 +3541,7 @@ RelationSetNewRelfilenode(Relation relation, char persistence) * Make the pg_class row change visible, as well as the relation map * change if any. This will cause the relcache entry to get updated, too. */ - CommandCounterIncrement(); + //CommandCounterIncrement(); /* * Mark the rel as having been given a new relfilenode in the current
