From fdc7efcc30b1da950ade72fd35852c21bf8de839 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 23 Apr 2019 21:42:12 -0700 Subject: [PATCH] Allow pg_class xid & multixid horizons to not be set. This allows table AMs that don't need these horizons. This was already documented in the tableam relation_set_new_filenode callback, but an assert prevented if from actually working (the test AM code contained the change itself). Defang the asserts in the general code, and move the stronger ones into heap AM. Relatedly, after CLUSTER/VACUUM, we'd always assign a relfrozenxid / relminmxid. Change the table_relation_copy_for_cluster() interface to allow the AM to overwrite the horizons that get set on the pg_class entry. This'd also in the future allow AMs like heap to compute a relfrozenxid during rewrite that's the table's actual minimum rather than a pre-determined value. Arguably it'd have been better to move the whole computation / setting of those values into the callback, but it seems likely that for other reasons it'd be better to be able to use one value to vacuum/cluster multiple tables (e.g. a toast's horizon shouldn't be different than the table's). Reported-By: Heikki Linnakangas Author: Andres Freund Discussion: https://postgr.es/m/9a7fb9cc-2419-5db7-8840-ddc10c93f122@iki.fi --- src/backend/access/heap/heapam_handler.c | 8 ++-- src/backend/access/heap/vacuumlazy.c | 4 ++ src/backend/commands/cluster.c | 24 ++++++----- src/backend/commands/vacuum.c | 53 +++++++++++++++++------- src/backend/postmaster/autovacuum.c | 4 +- src/include/access/tableam.h | 35 +++++++++------- 6 files changed, 83 insertions(+), 45 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 1d8ca2429f9..6584a9cb8da 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -668,8 +668,8 @@ static void heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, Relation OldIndex, bool use_sort, TransactionId OldestXmin, - TransactionId FreezeXid, - MultiXactId MultiXactCutoff, + TransactionId *xid_cutoff, + MultiXactId *multi_cutoff, double *num_tuples, double *tups_vacuumed, double *tups_recently_dead) @@ -707,8 +707,8 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, isnull = (bool *) palloc(natts * sizeof(bool)); /* Initialize the rewrite operation */ - rwstate = begin_heap_rewrite(OldHeap, NewHeap, OldestXmin, FreezeXid, - MultiXactCutoff, use_wal); + rwstate = begin_heap_rewrite(OldHeap, NewHeap, OldestXmin, *xid_cutoff, + *multi_cutoff, use_wal); /* Set up sorting if wanted */ diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 8dc76fa8583..9364cd4c33f 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -213,6 +213,10 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params, Assert(params != NULL); Assert(params->index_cleanup != VACOPT_TERNARY_DEFAULT); + /* not every AM requires these to be valid, but heap does */ + Assert(TransactionIdIsNormal(onerel->rd_rel->relfrozenxid)); + Assert(MultiXactIdIsValid(onerel->rd_rel->relminmxid)); + /* measure elapsed time iff autovacuum logging requires it */ if (IsAutoVacuumWorkerProcess() && params->log_min_duration >= 0) { diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 4f4be1efbfc..3ee70560476 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -870,19 +870,17 @@ copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose, * FreezeXid will become the table's new relfrozenxid, and that mustn't go * backwards, so take the max. */ - if (TransactionIdPrecedes(FreezeXid, OldHeap->rd_rel->relfrozenxid)) + if (TransactionIdIsValid(OldHeap->rd_rel->relfrozenxid) && + TransactionIdPrecedes(FreezeXid, OldHeap->rd_rel->relfrozenxid)) FreezeXid = OldHeap->rd_rel->relfrozenxid; /* * MultiXactCutoff, similarly, shouldn't go backwards either. */ - if (MultiXactIdPrecedes(MultiXactCutoff, OldHeap->rd_rel->relminmxid)) + if (MultiXactIdIsValid(OldHeap->rd_rel->relminmxid) && + MultiXactIdPrecedes(MultiXactCutoff, OldHeap->rd_rel->relminmxid)) MultiXactCutoff = OldHeap->rd_rel->relminmxid; - /* return selected values to caller */ - *pFreezeXid = FreezeXid; - *pCutoffMulti = MultiXactCutoff; - /* * Decide whether to use an indexscan or seqscan-and-optional-sort to scan * the OldHeap. We know how to use a sort to duplicate the ordering of a @@ -915,13 +913,19 @@ copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose, /* * Hand of the actual copying to AM specific function, the generic code - * cannot know how to deal with visibility across AMs. + * cannot know how to deal with visibility across AMs. Note that this + * routine is allowed to set FreezeXid / MultiXactCutoff to different + * values (e.g. because the AM doesn't use freezing). */ table_relation_copy_for_cluster(OldHeap, NewHeap, OldIndex, use_sort, - OldestXmin, FreezeXid, MultiXactCutoff, + OldestXmin, &FreezeXid, &MultiXactCutoff, &num_tuples, &tups_vacuumed, &tups_recently_dead); + /* return selected values to caller, get set as relfrozenxid/minmxid */ + *pFreezeXid = FreezeXid; + *pCutoffMulti = MultiXactCutoff; + /* Reset rd_toastoid just to be tidy --- it shouldn't be looked at again */ NewHeap->rd_toastoid = InvalidOid; @@ -1118,9 +1122,9 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class, /* set rel1's frozen Xid and minimum MultiXid */ if (relform1->relkind != RELKIND_INDEX) { - Assert(TransactionIdIsNormal(frozenXid)); + Assert(!TransactionIdIsValid(frozenXid) || + TransactionIdIsNormal(frozenXid)); relform1->relfrozenxid = frozenXid; - Assert(MultiXactIdIsValid(cutoffMulti)); relform1->relminmxid = cutoffMulti; } diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 1a7291d94bc..94fb6f26063 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -1313,36 +1313,61 @@ vac_update_datfrozenxid(void) /* * Only consider relations able to hold unfrozen XIDs (anything else - * should have InvalidTransactionId in relfrozenxid anyway.) + * should have InvalidTransactionId in relfrozenxid anyway). */ if (classForm->relkind != RELKIND_RELATION && classForm->relkind != RELKIND_MATVIEW && classForm->relkind != RELKIND_TOASTVALUE) + { + Assert(!TransactionIdIsValid(classForm->relfrozenxid)); + Assert(!MultiXactIdIsValid(classForm->relminmxid)); continue; - - Assert(TransactionIdIsNormal(classForm->relfrozenxid)); - Assert(MultiXactIdIsValid(classForm->relminmxid)); + } /* + * Some table AMs might not need per-relation xid / multixid + * horizons. It therefore seems reasonable to allow relfrozenxid and + * relminmxid to not be set (i.e. set to their respective Invalid*Id) + * independently. Thus validate and compute horizon for each only if + * set. + * * If things are working properly, no relation should have a * relfrozenxid or relminmxid that is "in the future". However, such * cases have been known to arise due to bugs in pg_upgrade. If we * see any entries that are "in the future", chicken out and don't do - * anything. This ensures we won't truncate clog before those - * relations have been scanned and cleaned up. + * anything. This ensures we won't truncate clog & multixact SLRUs + * before those relations have been scanned and cleaned up. */ - if (TransactionIdPrecedes(lastSaneFrozenXid, classForm->relfrozenxid) || - MultiXactIdPrecedes(lastSaneMinMulti, classForm->relminmxid)) + + if (TransactionIdIsValid(classForm->relfrozenxid)) { - bogus = true; - break; + Assert(TransactionIdIsNormal(classForm->relfrozenxid)); + + /* check for values in the future */ + if (TransactionIdPrecedes(lastSaneFrozenXid, classForm->relfrozenxid)) + { + bogus = true; + break; + } + + /* determine new horizon */ + if (TransactionIdPrecedes(classForm->relfrozenxid, newFrozenXid)) + newFrozenXid = classForm->relfrozenxid; } - if (TransactionIdPrecedes(classForm->relfrozenxid, newFrozenXid)) - newFrozenXid = classForm->relfrozenxid; + if (MultiXactIdIsValid(classForm->relminmxid)) + { + /* check for values in the future */ + if (MultiXactIdPrecedes(lastSaneMinMulti, classForm->relminmxid)) + { + bogus = true; + break; + } - if (MultiXactIdPrecedes(classForm->relminmxid, newMinMulti)) - newMinMulti = classForm->relminmxid; + /* determine new horizon */ + if (MultiXactIdPrecedes(classForm->relminmxid, newMinMulti)) + newMinMulti = classForm->relminmxid; + } } /* we're done with pg_class */ diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 0976029e737..53c91d92778 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -3033,8 +3033,8 @@ relation_needs_vacanalyze(Oid relid, multiForceLimit = recentMulti - multixact_freeze_max_age; if (multiForceLimit < FirstMultiXactId) multiForceLimit -= FirstMultiXactId; - force_vacuum = MultiXactIdPrecedes(classForm->relminmxid, - multiForceLimit); + force_vacuum = MultiXactIdIsValid(classForm->relminmxid) && + MultiXactIdPrecedes(classForm->relminmxid, multiForceLimit); } *wraparound = force_vacuum; diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 6fbfcb96c98..c018a44267a 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -452,8 +452,8 @@ typedef struct TableAmRoutine Relation OldIndex, bool use_sort, TransactionId OldestXmin, - TransactionId FreezeXid, - MultiXactId MultiXactCutoff, + TransactionId *xid_cutoff, + MultiXactId *multi_cutoff, double *num_tuples, double *tups_vacuumed, double *tups_recently_dead); @@ -1297,32 +1297,37 @@ table_relation_copy_data(Relation rel, RelFileNode newrnode) * Copy data from `OldHeap` into `NewHeap`, as part of a CLUSTER or VACUUM * FULL. * - * If `use_sort` is true, the table contents are sorted appropriate for - * `OldIndex`; if use_sort is false and OldIndex is not InvalidOid, the data - * is copied in that index's order; if use_sort is false and OidIndex is - * InvalidOid, no sorting is performed. + * Additional Input parameters: + * - use_sort - if true, the table contents are sorted appropriate for + * `OldIndex`; if false and OldIndex is not InvalidOid, the data is copied + * in that index's order; if false and OidIndex is InvalidOid, no sorting is + * performed + * - OidIndex - see use_sort + * - OldestXmin - computed by vacuum_set_xid_limits(), even when + * not needed for the relation's AM + * - *xid_cutoff - dito + * - *multi_cutoff - dito * - * OldestXmin, FreezeXid, MultiXactCutoff must be currently valid values for - * the table. - * - * *num_tuples, *tups_vacuumed, *tups_recently_dead will contain statistics - * computed while copying for the relation. Not all might make sense for every - * AM. + * Output parameters: + * - *xid_cutoff - rel's new relfrozenxid value, may be invalid + * - *multi_cutoff - rel's new relminmxid value, may be invalid + * - *tups_vacuumed - stats, for logging, if appropriate for AM + * - *tups_recently_dead - stats, for logging, if appropriate for AM */ static inline void table_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, Relation OldIndex, bool use_sort, TransactionId OldestXmin, - TransactionId FreezeXid, - MultiXactId MultiXactCutoff, + TransactionId *xid_cutoff, + MultiXactId *multi_cutoff, double *num_tuples, double *tups_vacuumed, double *tups_recently_dead) { OldHeap->rd_tableam->relation_copy_for_cluster(OldHeap, NewHeap, OldIndex, use_sort, OldestXmin, - FreezeXid, MultiXactCutoff, + xid_cutoff, multi_cutoff, num_tuples, tups_vacuumed, tups_recently_dead); } -- 2.39.5