From 3d0a4636aa4c976e971c05c77e162fc70c61f40b Mon Sep 17 00:00:00 2001 From: Andres Freund <andres@anarazel.de> Date: Sat, 24 Jul 2021 20:14:03 -0700 Subject: [PATCH] Deduplicate choice of horizon for a relation procarray.c. 5a1e1d83022 was a minimal bug fix for dc7420c2c92. To avoid future bugs of that kind, deduplicate the choice of a relation's horizon into a new helper, GlobalVisHorizonKindForRel(). As the code in question was only introduced in dc7420c2c92 it seems worth backpatching this change as well, otherwise 14 will look different from all other branches. A different approach to this was suggested by Matthias van de Meent. Author: Andres Freund Discussion: https://postgr.es/m/20210621122919.2qhu3pfugxxp3cji@alap3.anarazel.de Backpatch: 14, like 5a1e1d83022 --- src/backend/storage/ipc/procarray.c | 98 +++++++++++++++++++---------- 1 file changed, 64 insertions(+), 34 deletions(-) diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 793df973b4..aa3e1419a9 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -246,6 +246,17 @@ typedef struct ComputeXidHorizonsResult } ComputeXidHorizonsResult; +/* + * Return value for GlobalVisHorizonKindForRel(). + */ +typedef enum GlobalVisHorizonKind +{ + VISHORIZON_SHARED, + VISHORIZON_CATALOG, + VISHORIZON_DATA, + VISHORIZON_TEMP +} GlobalVisHorizonKind; + static ProcArrayStruct *procArray; @@ -1952,6 +1963,33 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h) GlobalVisUpdateApply(h); } +/* + * Determine what kind of visibility horizon needs to be used for a + * relation. If rel is NULL, the most conservative horizon is used. + */ +static inline GlobalVisHorizonKind +GlobalVisHorizonKindForRel(Relation rel) +{ + /* + * Other relkkinds currently don't contain xids, nor always the necessary + * logical decoding markers. + */ + Assert(!rel || + rel->rd_rel->relkind == RELKIND_RELATION || + rel->rd_rel->relkind == RELKIND_MATVIEW || + rel->rd_rel->relkind == RELKIND_TOASTVALUE); + + if (rel == NULL || rel->rd_rel->relisshared || RecoveryInProgress()) + return VISHORIZON_SHARED; + else if (IsCatalogRelation(rel) || + RelationIsAccessibleInLogicalDecoding(rel)) + return VISHORIZON_CATALOG; + else if (!RELATION_IS_LOCAL(rel)) + return VISHORIZON_DATA; + else + return VISHORIZON_TEMP; +} + /* * Return the oldest XID for which deleted tuples must be preserved in the * passed table. @@ -1970,16 +2008,19 @@ GetOldestNonRemovableTransactionId(Relation rel) ComputeXidHorizons(&horizons); - /* select horizon appropriate for relation */ - if (rel == NULL || rel->rd_rel->relisshared || RecoveryInProgress()) - return horizons.shared_oldest_nonremovable; - else if (IsCatalogRelation(rel) || - RelationIsAccessibleInLogicalDecoding(rel)) - return horizons.catalog_oldest_nonremovable; - else if (RELATION_IS_LOCAL(rel)) - return horizons.temp_oldest_nonremovable; - else - return horizons.data_oldest_nonremovable; + switch (GlobalVisHorizonKindForRel(rel)) + { + case VISHORIZON_SHARED: + return horizons.shared_oldest_nonremovable; + case VISHORIZON_CATALOG: + return horizons.catalog_oldest_nonremovable; + case VISHORIZON_DATA: + return horizons.data_oldest_nonremovable; + case VISHORIZON_TEMP: + return horizons.temp_oldest_nonremovable; + } + + return InvalidTransactionId; } /* @@ -3986,38 +4027,27 @@ DisplayXidCache(void) GlobalVisState * GlobalVisTestFor(Relation rel) { - bool need_shared; - bool need_catalog; GlobalVisState *state; /* XXX: we should assert that a snapshot is pushed or registered */ Assert(RecentXmin); - if (!rel) - need_shared = need_catalog = true; - else + switch (GlobalVisHorizonKindForRel(rel)) { - /* - * Other kinds currently don't contain xids, nor always the necessary - * logical decoding markers. - */ - Assert(rel->rd_rel->relkind == RELKIND_RELATION || - rel->rd_rel->relkind == RELKIND_MATVIEW || - rel->rd_rel->relkind == RELKIND_TOASTVALUE); - - need_shared = rel->rd_rel->relisshared || RecoveryInProgress(); - need_catalog = IsCatalogRelation(rel) || RelationIsAccessibleInLogicalDecoding(rel); + case VISHORIZON_SHARED: + state = &GlobalVisSharedRels; + break; + case VISHORIZON_CATALOG: + state = &GlobalVisCatalogRels; + break; + case VISHORIZON_DATA: + state = &GlobalVisDataRels; + break; + case VISHORIZON_TEMP: + state = &GlobalVisTempRels; + break; } - if (need_shared) - state = &GlobalVisSharedRels; - else if (need_catalog) - state = &GlobalVisCatalogRels; - else if (RELATION_IS_LOCAL(rel)) - state = &GlobalVisTempRels; - else - state = &GlobalVisDataRels; - Assert(FullTransactionIdIsValid(state->definitely_needed) && FullTransactionIdIsValid(state->maybe_needed)); -- 2.24.1