From a22780e3544d7c83b5ad8851de240c3d5ef8f221 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Sat, 29 Feb 2020 18:07:25 -0800 Subject: [PATCH v1 1/2] TMP: work around missing snapshot registrations. This is just what's hit by the tests. It's not an actual fix. --- src/backend/catalog/namespace.c | 7 +++++++ src/backend/catalog/pg_subscription.c | 4 ++++ src/backend/commands/indexcmds.c | 9 +++++++++ src/backend/commands/tablecmds.c | 8 ++++++++ src/backend/replication/logical/tablesync.c | 12 ++++++++++++ src/backend/replication/logical/worker.c | 4 ++++ src/backend/utils/time/snapmgr.c | 4 ++++ 7 files changed, 48 insertions(+) diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 2ec23016fe5..e4696d8d417 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -55,6 +55,7 @@ #include "utils/inval.h" #include "utils/lsyscache.h" #include "utils/memutils.h" +#include "utils/snapmgr.h" #include "utils/syscache.h" #include "utils/varlena.h" @@ -4244,12 +4245,18 @@ RemoveTempRelationsCallback(int code, Datum arg) { if (OidIsValid(myTempNamespace)) /* should always be true */ { + Snapshot snap; + /* Need to ensure we have a usable transaction. */ AbortOutOfAnyTransaction(); StartTransactionCommand(); + /* ensure xmin stays set */ + snap = RegisterSnapshot(GetCatalogSnapshot(InvalidOid)); + RemoveTempRelations(myTempNamespace); + UnregisterSnapshot(snap); CommitTransactionCommand(); } } diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c index cb157311154..4a324dfb4f1 100644 --- a/src/backend/catalog/pg_subscription.c +++ b/src/backend/catalog/pg_subscription.c @@ -31,6 +31,7 @@ #include "utils/fmgroids.h" #include "utils/pg_lsn.h" #include "utils/rel.h" +#include "utils/snapmgr.h" #include "utils/syscache.h" static List *textarray_to_stringlist(ArrayType *textarray); @@ -286,6 +287,7 @@ UpdateSubscriptionRelState(Oid subid, Oid relid, char state, bool nulls[Natts_pg_subscription_rel]; Datum values[Natts_pg_subscription_rel]; bool replaces[Natts_pg_subscription_rel]; + Snapshot snap = RegisterSnapshot(GetCatalogSnapshot(InvalidOid)); LockSharedObject(SubscriptionRelationId, subid, 0, AccessShareLock); @@ -321,6 +323,8 @@ UpdateSubscriptionRelState(Oid subid, Oid relid, char state, /* Cleanup. */ table_close(rel, NoLock); + + UnregisterSnapshot(snap); } /* diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 4e8263af4be..b5fe3649a22 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -2720,6 +2720,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) char *relationName = NULL; char *relationNamespace = NULL; PGRUsage ru0; + Snapshot snap; /* * Create a memory context that will survive forced transaction commits we @@ -3189,6 +3190,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) */ StartTransactionCommand(); + snap = RegisterSnapshot(GetCatalogSnapshot(InvalidOid)); forboth(lc, indexIds, lc2, newIndexIds) { @@ -3237,8 +3239,11 @@ ReindexRelationConcurrently(Oid relationOid, int options) } /* Commit this transaction and make index swaps visible */ + UnregisterSnapshot(snap); CommitTransactionCommand(); + StartTransactionCommand(); + snap = RegisterSnapshot(GetCatalogSnapshot(InvalidOid)); /* * Phase 5 of REINDEX CONCURRENTLY @@ -3269,7 +3274,9 @@ ReindexRelationConcurrently(Oid relationOid, int options) } /* Commit this transaction to make the updates visible. */ + UnregisterSnapshot(snap); CommitTransactionCommand(); + StartTransactionCommand(); /* @@ -3283,6 +3290,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) WaitForLockersMultiple(lockTags, AccessExclusiveLock, true); PushActiveSnapshot(GetTransactionSnapshot()); + snap = RegisterSnapshot(GetCatalogSnapshot(InvalidOid)); { ObjectAddresses *objects = new_object_addresses(); @@ -3308,6 +3316,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) } PopActiveSnapshot(); + UnregisterSnapshot(snap); CommitTransactionCommand(); /* diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 8e35c5bd1a2..311a950297a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -15148,6 +15148,7 @@ PreCommit_on_commit_actions(void) ListCell *l; List *oids_to_truncate = NIL; List *oids_to_drop = NIL; + Snapshot snap; foreach(l, on_commits) { @@ -15179,6 +15180,11 @@ PreCommit_on_commit_actions(void) } } + if (oids_to_truncate == NIL && oids_to_drop == NIL) + return; + + snap = RegisterSnapshot(GetCatalogSnapshot(InvalidOid)); + /* * Truncate relations before dropping so that all dependencies between * relations are removed after they are worked on. Doing it like this @@ -15232,6 +15238,8 @@ PreCommit_on_commit_actions(void) } #endif } + + UnregisterSnapshot(snap); } /* diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index a60c6661538..5bdb15b1d50 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -864,6 +864,7 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos) { Relation rel; WalRcvExecResult *res; + Snapshot snap; SpinLockAcquire(&MyLogicalRepWorker->relmutex); MyLogicalRepWorker->relstate = SUBREL_STATE_DATASYNC; @@ -872,10 +873,14 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos) /* Update the state and make it visible to others. */ StartTransactionCommand(); + snap = RegisterSnapshot(GetCatalogSnapshot(InvalidOid)); + UpdateSubscriptionRelState(MyLogicalRepWorker->subid, MyLogicalRepWorker->relid, MyLogicalRepWorker->relstate, MyLogicalRepWorker->relstate_lsn); + + UnregisterSnapshot(snap); CommitTransactionCommand(); pgstat_report_stat(false); @@ -919,6 +924,7 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos) CRS_USE_SNAPSHOT, origin_startpos); PushActiveSnapshot(GetTransactionSnapshot()); + snap = RegisterSnapshot(GetCatalogSnapshot(InvalidOid)); copy_table(rel); PopActiveSnapshot(); @@ -934,6 +940,8 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos) /* Make the copy visible. */ CommandCounterIncrement(); + UnregisterSnapshot(snap); + /* * We are done with the initial data synchronization, update * the state. @@ -958,6 +966,8 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos) */ if (*origin_startpos >= MyLogicalRepWorker->relstate_lsn) { + snap = RegisterSnapshot(GetCatalogSnapshot(InvalidOid)); + /* * Update the new state in catalog. No need to bother * with the shmem state as we are exiting for good. @@ -966,6 +976,8 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos) MyLogicalRepWorker->relid, SUBREL_STATE_SYNCDONE, *origin_startpos); + UnregisterSnapshot(snap); + finish_sync_worker(); } break; diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index fa3811783f6..f60b1581abf 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -989,6 +989,9 @@ apply_handle_truncate(StringInfo s) ensure_transaction(); + /* catalog modifications need a set snapshot */ + PushActiveSnapshot(GetTransactionSnapshot()); + remote_relids = logicalrep_read_truncate(s, &cascade, &restart_seqs); foreach(lc, remote_relids) @@ -1029,6 +1032,7 @@ apply_handle_truncate(StringInfo s) } CommandCounterIncrement(); + PopActiveSnapshot(); } diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 1c063c592ce..b5cff157bf6 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -441,6 +441,8 @@ GetOldestSnapshot(void) Snapshot GetCatalogSnapshot(Oid relid) { + Assert(IsTransactionState()); + /* * Return historic snapshot while we're doing logical decoding, so we can * see the appropriate state of the catalog. @@ -1017,6 +1019,8 @@ SnapshotResetXmin(void) if (pairingheap_is_empty(&RegisteredSnapshots)) { MyPgXact->xmin = InvalidTransactionId; + TransactionXmin = InvalidTransactionId; + RecentXmin = InvalidTransactionId; return; } -- 2.25.0.114.g5b0ca878e0