From 85b506bbfc2937c9abdfcce4e01a8feca8e64ee8 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Sat, 15 Nov 2014 01:19:49 -0300 Subject: [PATCH] Get rid of SET LOGGED indexes persistence kludge MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This removes ATChangeIndexesPersistence() introduced by f41872d0c1239d36 which was too ugly to live for long. Instead, the correct persistence marking is passed all the way down to reindex_index, so that the transient relation built to contain the index relfilenode can get marked correctly right from the start. Author: Fabrízio de Royes Mello Review and editorialization by Michael Paquier and Álvaro Herrera --- src/backend/catalog/index.c | 28 +++++++++++++-- src/backend/commands/cluster.c | 16 +++++++-- src/backend/commands/indexcmds.c | 2 +- src/backend/commands/matview.c | 8 ++--- src/backend/commands/tablecmds.c | 56 ++---------------------------- src/backend/utils/cache/relcache.c | 1 + src/include/catalog/index.h | 11 +++--- src/include/commands/cluster.h | 3 +- 8 files changed, 56 insertions(+), 69 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 912038a712..b57aa95faf 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1980,7 +1980,7 @@ index_build(Relation heapRelation, * created it, or truncated twice in a subsequent transaction, the * relfilenode won't change, and nothing needs to be done here. */ - if (heapRelation->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED && + if (indexRelation->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED && !smgrexists(indexRelation->rd_smgr, INIT_FORKNUM)) { RegProcedure ambuildempty = indexRelation->rd_am->ambuildempty; @@ -3130,7 +3130,7 @@ IndexGetRelation(Oid indexId, bool missing_ok) * reindex_index - This routine is used to recreate a single index */ void -reindex_index(Oid indexId, bool skip_constraint_checks) +reindex_index(Oid indexId, bool skip_constraint_checks, char persistence) { Relation iRel, heapRelation; @@ -3191,6 +3191,9 @@ reindex_index(Oid indexId, bool skip_constraint_checks) indexInfo->ii_ExclusionStrats = NULL; } + /* Set the relpersistence of the new index */ + iRel->rd_rel->relpersistence = persistence; + /* We'll build a new physical relation for the index */ RelationSetNewRelfilenode(iRel, InvalidTransactionId, InvalidMultiXactId); @@ -3310,6 +3313,12 @@ reindex_index(Oid indexId, bool skip_constraint_checks) * performance, other callers should include the flag only after transforming * the data in a manner that risks a change in constraint validity. * + * REINDEX_REL_FORCE_INDEXES_UNLOGGED: if true, set the persistence of the + * rebuilt indexes to unlogged. + * + * REINDEX_REL_FORCE_INDEXES_LOGGED: if true, set the persistence of the + * rebuilt indexes to permanent. + * * Returns true if any indexes were rebuilt (including toast table's index * when relevant). Note that a CommandCounterIncrement will occur after each * index rebuild. @@ -3371,6 +3380,7 @@ reindex_relation(Oid relid, int flags) { List *doneIndexes; ListCell *indexId; + char persistence; if (flags & REINDEX_REL_SUPPRESS_INDEX_USE) { @@ -3384,6 +3394,17 @@ reindex_relation(Oid relid, int flags) CommandCounterIncrement(); } + /* + * Compute persistence of indexes: same as that of owning rel, unless + * caller specified otherwise. + */ + if (flags & REINDEX_REL_FORCE_INDEXES_UNLOGGED) + persistence = RELPERSISTENCE_UNLOGGED; + else if (flags & REINDEX_REL_FORCE_INDEXES_PERMANENT) + persistence = RELPERSISTENCE_PERMANENT; + else + persistence = rel->rd_rel->relpersistence; + /* Reindex all the indexes. */ doneIndexes = NIL; foreach(indexId, indexIds) @@ -3393,7 +3414,8 @@ reindex_relation(Oid relid, int flags) if (is_pg_class) RelationSetIndexList(rel, doneIndexes, InvalidOid); - reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS)); + reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS), + persistence); CommandCounterIncrement(); diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 6a578ec58f..bc5f33fb78 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -589,7 +589,8 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose) */ finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog, swap_toast_by_content, false, true, - frozenXid, cutoffMulti); + frozenXid, cutoffMulti, + OldHeap->rd_rel->relpersistence); } @@ -1475,7 +1476,8 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, bool check_constraints, bool is_internal, TransactionId frozenXid, - MultiXactId cutoffMulti) + MultiXactId cutoffMulti, + char newrelpersistence) { ObjectAddress object; Oid mapped_tables[4]; @@ -1519,6 +1521,16 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, reindex_flags = REINDEX_REL_SUPPRESS_INDEX_USE; if (check_constraints) reindex_flags |= REINDEX_REL_CHECK_CONSTRAINTS; + + /* + * Ensure that the indexes have the same persistence as the parent + * relation. + */ + if (newrelpersistence == RELPERSISTENCE_UNLOGGED) + reindex_flags |= REINDEX_REL_FORCE_INDEXES_UNLOGGED; + else if (newrelpersistence == RELPERSISTENCE_PERMANENT) + reindex_flags |= REINDEX_REL_FORCE_INDEXES_PERMANENT; + reindex_relation(OIDOldHeap, reindex_flags); /* diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 02055950b5..12b4ac7b3c 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1689,7 +1689,7 @@ ReindexIndex(RangeVar *indexRelation) RangeVarCallbackForReindexIndex, (void *) &heapOid); - reindex_index(indOid, false); + reindex_index(indOid, false, indexRelation->relpersistence); return indOid; } diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index 523ba35ba2..b19da4ac63 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -67,7 +67,7 @@ static void mv_GenerateOper(StringInfo buf, Oid opoid); static void refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, int save_sec_context); -static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap); +static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence); static void OpenMatViewIncrementalMaintenance(void); static void CloseMatViewIncrementalMaintenance(void); @@ -303,7 +303,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, Assert(matview_maintenance_depth == old_depth); } else - refresh_by_heap_swap(matviewOid, OIDNewHeap); + refresh_by_heap_swap(matviewOid, OIDNewHeap, relpersistence); /* Roll back any GUC changes */ AtEOXact_GUC(false, save_nestlevel); @@ -759,10 +759,10 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, * swapping is handled by the called function, so it is not needed here. */ static void -refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap) +refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence) { finish_heap_swap(matviewOid, OIDNewHeap, false, false, true, true, - RecentXmin, ReadNextMultiXactId()); + RecentXmin, ReadNextMultiXactId(), relpersistence); } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 714a9f1ee7..093224f4e6 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -393,7 +393,6 @@ static void ATExecClusterOn(Relation rel, const char *indexName, LOCKMODE lockmode); static void ATExecDropCluster(Relation rel, LOCKMODE lockmode); static bool ATPrepChangePersistence(Relation rel, bool toLogged); -static void ATChangeIndexesPersistence(Oid relid, char relpersistence); static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, char *tablespacename, LOCKMODE lockmode); static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode); @@ -3734,16 +3733,6 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode) */ ATRewriteTable(tab, OIDNewHeap, lockmode); - /* - * Change the persistence marking of indexes, if necessary. This - * is so that the new copies are built with the right persistence - * in the reindex step below. Note we cannot do this earlier, - * because the rewrite step might read the indexes, and that would - * cause buffers for them to have the wrong setting. - */ - if (tab->chgPersistence) - ATChangeIndexesPersistence(tab->relid, tab->newrelpersistence); - /* * Swap the physical files of the old and new heaps, then rebuild * indexes and discard the old heap. We can use RecentXmin for @@ -3756,7 +3745,8 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode) false, false, true, !OidIsValid(tab->newTableSpace), RecentXmin, - ReadNextMultiXactId()); + ReadNextMultiXactId(), + persistence); } else { @@ -10879,48 +10869,6 @@ ATPrepChangePersistence(Relation rel, bool toLogged) return true; } -/* - * Update the pg_class entry of each index for the given relation to the - * given persistence. - */ -static void -ATChangeIndexesPersistence(Oid relid, char relpersistence) -{ - Relation rel; - Relation pg_class; - List *indexes; - ListCell *cell; - - pg_class = heap_open(RelationRelationId, RowExclusiveLock); - - /* We already have a lock on the table */ - rel = relation_open(relid, NoLock); - indexes = RelationGetIndexList(rel); - foreach(cell, indexes) - { - Oid indexid = lfirst_oid(cell); - HeapTuple tuple; - Form_pg_class pg_class_form; - - tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexid)); - if (!HeapTupleIsValid(tuple)) - elog(ERROR, "cache lookup failed for relation %u", - indexid); - - pg_class_form = (Form_pg_class) GETSTRUCT(tuple); - pg_class_form->relpersistence = relpersistence; - simple_heap_update(pg_class, &tuple->t_self, tuple); - - /* keep catalog indexes current */ - CatalogUpdateIndexes(pg_class, tuple); - - heap_freetuple(tuple); - } - - heap_close(pg_class, RowExclusiveLock); - heap_close(rel, NoLock); -} - /* * Execute ALTER TABLE SET SCHEMA */ diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 4b4528f791..2250c56d28 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -3078,6 +3078,7 @@ RelationSetNewRelfilenode(Relation relation, TransactionId freezeXid, } classform->relfrozenxid = freezeXid; classform->relminmxid = minmulti; + classform->relpersistence = relation->rd_rel->relpersistence; simple_heap_update(pg_class, &tuple->t_self, tuple); CatalogUpdateIndexes(pg_class, tuple); diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index c36a729c91..199f2d7834 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -111,12 +111,15 @@ extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot); extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action); -extern void reindex_index(Oid indexId, bool skip_constraint_checks); +extern void reindex_index(Oid indexId, bool skip_constraint_checks, + char relpersistence); /* Flag bits for reindex_relation(): */ -#define REINDEX_REL_PROCESS_TOAST 0x01 -#define REINDEX_REL_SUPPRESS_INDEX_USE 0x02 -#define REINDEX_REL_CHECK_CONSTRAINTS 0x04 +#define REINDEX_REL_PROCESS_TOAST 0x01 +#define REINDEX_REL_SUPPRESS_INDEX_USE 0x02 +#define REINDEX_REL_CHECK_CONSTRAINTS 0x04 +#define REINDEX_REL_FORCE_INDEXES_UNLOGGED 0x08 +#define REINDEX_REL_FORCE_INDEXES_PERMANENT 0x10 extern bool reindex_relation(Oid relid, int flags); diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index f7730a9c03..0b7e87798e 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -33,6 +33,7 @@ extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, bool check_constraints, bool is_internal, TransactionId frozenXid, - MultiXactId minMulti); + MultiXactId minMulti, + char newrelpersistence); #endif /* CLUSTER_H */ -- 2.40.0