From 04009b1f070cd1cd8c9c8ed8efaecc768dcb9063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Nordstr=C3=B6m?= Date: Wed, 15 Jan 2025 15:25:29 +0100 Subject: [PATCH] Refactor vacuum cutoffs compatibility functions The compatibility layer in TimescaleDB consolidates the API differences around getting the vacuum cutoffs (used by, e.g., TimescaleDB's reorder) based on an old API. Since PostgreSQL 17 introduced a new function API to get the vacuum cutoffs, this change refactors the compitability layer to provide the new API for older PostgreSQL versions. --- src/compat/compat.h | 108 ++++++++++++++++++-------------------------- tsl/src/reorder.c | 37 +++++++++------ 2 files changed, 69 insertions(+), 76 deletions(-) diff --git a/src/compat/compat.h b/src/compat/compat.h index 58654de9f9e..a5e66e47efb 100644 --- a/src/compat/compat.h +++ b/src/compat/compat.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -417,74 +418,54 @@ pg_strtoint64(const char *str) check_index_is_clusterable(rel, indexOid, true, lock) #endif +#if PG16_LT /* * PG15 consolidate VACUUM xid cutoff logic. * * https://github.com/postgres/postgres/commit/efa4a946 + * + * PG16 introduced VacuumCutoffs so define here for previous PG versions. */ -#if PG15_LT -#define vacuum_set_xid_limits_compat(rel, \ - freeze_min_age, \ - freeze_table_age, \ - multixact_freeze_min_age, \ - multixact_freeze_table_age, \ - oldestXmin, \ - freezeLimit, \ - multiXactCutoff) \ - vacuum_set_xid_limits(rel, \ - freeze_min_age, \ - freeze_table_age, \ - multixact_freeze_min_age, \ - multixact_freeze_table_age, \ - oldestXmin, \ - freezeLimit, \ - NULL, \ - multiXactCutoff, \ - NULL) -#elif PG16_LT -#define vacuum_set_xid_limits_compat(rel, \ - freeze_min_age, \ - freeze_table_age, \ - multixact_freeze_min_age, \ - multixact_freeze_table_age, \ - oldestXmin, \ - freezeLimit, \ - multiXactCutoff) \ - do \ - { \ - MultiXactId oldestMxact; \ - vacuum_set_xid_limits(rel, \ - freeze_min_age, \ - freeze_table_age, \ - multixact_freeze_min_age, \ - multixact_freeze_table_age, \ - oldestXmin, \ - &oldestMxact, \ - freezeLimit, \ - multiXactCutoff); \ - } while (0) -#else -#define vacuum_set_xid_limits_compat(rel, \ - freezeMinAge, \ - freezeTableAge, \ - multixactFreezeMinAge, \ - multixactFreezeTableAge, \ - oldestXmin, \ - freezeLimit, \ - multiXactCutoff) \ - do \ - { \ - struct VacuumCutoffs cutoffs; \ - /* vacuum_get_cutoffs uses only the *_age members of the VacuumParams object */ \ - VacuumParams params = { .freeze_min_age = freezeMinAge, \ - .freeze_table_age = freezeTableAge, \ - .multixact_freeze_min_age = multixactFreezeMinAge, \ - .multixact_freeze_table_age = multixactFreezeTableAge }; \ - vacuum_get_cutoffs(rel, ¶ms, &cutoffs); \ - *(oldestXmin) = cutoffs.OldestXmin; \ - *(freezeLimit) = cutoffs.FreezeLimit; \ - *(multiXactCutoff) = cutoffs.MultiXactCutoff; \ - } while (0) +struct VacuumCutoffs +{ + TransactionId relfrozenxid; + MultiXactId relminmxid; + TransactionId OldestXmin; + MultiXactId OldestMxact; + TransactionId FreezeLimit; + MultiXactId MultiXactCutoff; +}; + +static inline bool +vacuum_get_cutoffs(Relation rel, const VacuumParams *params, struct VacuumCutoffs *cutoffs) +{ +#if PG15 + return vacuum_set_xid_limits(rel, + 0, + 0, + 0, + 0, + &cutoffs->OldestXmin, + &cutoffs->OldestMxact, + &cutoffs->FreezeLimit, + &cutoffs->MultiXactCutoff); +#elif PG14 + vacuum_set_xid_limits(rel, + 0, + 0, + 0, + 0, + &cutoffs->OldestXmin, + &cutoffs->FreezeLimit, + NULL, + &cutoffs->MultiXactCutoff, + NULL); + + /* Should aggressive vacuum be done? PG14 doesn't support the return value + * so return false. */ + return false; +#endif +} #endif /* @@ -977,3 +958,4 @@ RestrictSearchPath(void) is_internal, \ constraintId) #endif + diff --git a/tsl/src/reorder.c b/tsl/src/reorder.c index f9c2b4e3c61..87f99bf266c 100644 --- a/tsl/src/reorder.c +++ b/tsl/src/reorder.c @@ -534,9 +534,6 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose, int natts; Datum *values; bool *isnull; - TransactionId OldestXmin; - TransactionId FreezeXid; - MultiXactId MultiXactCutoff; bool use_sort; double num_tuples = 0, tups_vacuumed = 0, tups_recently_dead = 0; BlockNumber num_pages; @@ -626,24 +623,38 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose, * Since we're going to rewrite the whole table anyway, there's no reason * not to be aggressive about this. */ - vacuum_set_xid_limits_compat(OldHeap, 0, 0, 0, 0, &OldestXmin, &FreezeXid, &MultiXactCutoff); + struct VacuumCutoffs cutoffs; + VacuumParams params; + + memset(¶ms, 0, sizeof(VacuumParams)); + vacuum_get_cutoffs(OldHeap, ¶ms, &cutoffs); /* * 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)) - FreezeXid = OldHeap->rd_rel->relfrozenxid; + { + TransactionId relfrozenxid = OldHeap->rd_rel->relfrozenxid; + + if (TransactionIdIsValid(relfrozenxid) && + TransactionIdPrecedes(cutoffs.FreezeLimit, relfrozenxid)) + cutoffs.FreezeLimit = relfrozenxid; + } /* * MultiXactCutoff, similarly, shouldn't go backwards either. */ - if (MultiXactIdPrecedes(MultiXactCutoff, OldHeap->rd_rel->relminmxid)) - MultiXactCutoff = OldHeap->rd_rel->relminmxid; + { + MultiXactId relminmxid = OldHeap->rd_rel->relminmxid; + + if (MultiXactIdIsValid(relminmxid) && + MultiXactIdPrecedes(cutoffs.MultiXactCutoff, relminmxid)) + cutoffs.MultiXactCutoff = relminmxid; + } /* return selected values to caller */ - *pFreezeXid = FreezeXid; - *pCutoffMulti = MultiXactCutoff; + *pFreezeXid = cutoffs.FreezeLimit; + *pCutoffMulti = cutoffs.MultiXactCutoff; /* * We know how to use a sort to duplicate the ordering of a btree index, @@ -677,9 +688,9 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose, NewHeap, OldIndex, use_sort, - OldestXmin, - &FreezeXid, - &MultiXactCutoff, + cutoffs.OldestXmin, + &cutoffs.FreezeLimit, + &cutoffs.MultiXactCutoff, &num_tuples, &tups_vacuumed, &tups_recently_dead);