From fe5cb0430a06a44823d5b62f2f909da624505962 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent Date: Wed, 9 Jun 2021 17:26:34 +0200 Subject: [PATCH v1] Fix a bug in GetOldestNonRemovableTransactionId GetOldestNonRemovableTransactionId(rel) did not return values consistent with GlobalVisTestFor(rel). This is now updated, and some assertions are added to ensure this problem case does not return. --- src/backend/storage/ipc/procarray.c | 39 +++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 42a89fc5dc..7177d50351 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -1944,18 +1944,25 @@ TransactionId GetOldestNonRemovableTransactionId(Relation rel) { ComputeXidHorizonsResult horizons; + TransactionId result; ComputeXidHorizons(&horizons); - /* select horizon appropriate for relation */ - if (rel == NULL || rel->rd_rel->relisshared) - return horizons.shared_oldest_nonremovable; - else if (RelationIsAccessibleInLogicalDecoding(rel)) - return horizons.catalog_oldest_nonremovable; + /* select horizon appropriate for relation. Mirrored from GlobalVisTestFor */ + if (rel == NULL || rel->rd_rel->relisshared || RecoveryInProgress()) + result = horizons.shared_oldest_nonremovable; + else if (IsCatalogRelation(rel) || RelationIsAccessibleInLogicalDecoding(rel)) + result = horizons.catalog_oldest_nonremovable; else if (RELATION_IS_LOCAL(rel)) - return horizons.temp_oldest_nonremovable; + result = horizons.temp_oldest_nonremovable; else - return horizons.data_oldest_nonremovable; + result = horizons.data_oldest_nonremovable; + + /* Sanity check */ + Assert(TransactionIdEquals( + XidFromFullTransactionId(GlobalVisTestFor(rel)->maybe_needed), + result)); + return result; } /* @@ -4032,6 +4039,24 @@ GlobalVisTestShouldUpdate(GlobalVisState *state) static void GlobalVisUpdateApply(ComputeXidHorizonsResult *horizons) { + /* assert non-decreasing nature of horizons */ + Assert(FullTransactionIdFollowsOrEquals( + FullXidRelativeTo(horizons->latest_completed, + horizons->shared_oldest_nonremovable), + GlobalVisSharedRels.maybe_needed)); + Assert(FullTransactionIdFollowsOrEquals( + FullXidRelativeTo(horizons->latest_completed, + horizons->catalog_oldest_nonremovable), + GlobalVisCatalogRels.maybe_needed)); + Assert(FullTransactionIdFollowsOrEquals( + FullXidRelativeTo(horizons->latest_completed, + horizons->data_oldest_nonremovable), + GlobalVisDataRels.maybe_needed)); + Assert(FullTransactionIdFollowsOrEquals( + FullXidRelativeTo(horizons->latest_completed, + horizons->temp_oldest_nonremovable), + GlobalVisTempRels.maybe_needed)); + GlobalVisSharedRels.maybe_needed = FullXidRelativeTo(horizons->latest_completed, horizons->shared_oldest_nonremovable); -- 2.20.1