From b9444fd22d8dcc05e3a27817943be7f5296503fa Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Mon, 14 Oct 2024 14:43:26 -0500 Subject: [PATCH v4 1/3] Ensure we have a snapshot when updating various system catalogs. Reported-by: Alexander Lakhin Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/18127-fe54b6a667f29658%40postgresql.org Discussion: https://postgr.es/m/18309-c0bf914950c46692%40postgresql.org Discussion: https://postgr.es/m/ZvMSUPOqUU-VNADN%40nathan Backpatch-through: FIXME --- src/backend/commands/tablecmds.c | 13 +++++++++++ src/backend/replication/logical/tablesync.c | 21 ++++++++++++++++++ src/backend/replication/logical/worker.c | 24 +++++++++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1ccc80087c..c6f82e93b9 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -19283,9 +19283,22 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, tab->rel = rel; } + /* + * Detaching the partition might involve TOAST table access, so ensure we + * have a valid snapshot. We only expect to get here without a snapshot + * in the concurrent path. + */ + if (concurrent) + PushActiveSnapshot(GetTransactionSnapshot()); + else + Assert(HaveRegisteredOrActiveSnapshot()); + /* Do the final part of detaching */ DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid); + if (concurrent) + PopActiveSnapshot(); + ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); /* keep our lock until commit */ diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index e03e761392..d3fbb18438 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -377,6 +377,12 @@ process_syncing_tables_for_sync(XLogRecPtr current_lsn) replorigin_session_origin_lsn = InvalidXLogRecPtr; replorigin_session_origin_timestamp = 0; + /* + * Updating pg_replication_origin might involve TOAST table access, so + * ensure we have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Drop the tablesync's origin tracking if exists. * @@ -387,6 +393,8 @@ process_syncing_tables_for_sync(XLogRecPtr current_lsn) */ replorigin_drop_by_name(originname, true, false); + PopActiveSnapshot(); + finish_sync_worker(); } else @@ -483,6 +491,12 @@ process_syncing_tables_for_apply(XLogRecPtr current_lsn) started_tx = true; } + /* + * Updating pg_replication_origin might involve TOAST table + * access, so ensure we have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Remove the tablesync origin tracking if exists. * @@ -500,6 +514,8 @@ process_syncing_tables_for_apply(XLogRecPtr current_lsn) sizeof(originname)); replorigin_drop_by_name(originname, true, false); + PopActiveSnapshot(); + /* * Update the state to READY only after the origin cleanup. */ @@ -1455,8 +1471,13 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos) * Then advance to the LSN got from walrcv_create_slot. This is WAL * logged for the purpose of recovery. Locks are to prevent the * replication origin from vanishing while advancing. + * + * Updating pg_replication_origin might involve TOAST table access, so + * ensure we have a valid snapshot. */ + PushActiveSnapshot(GetTransactionSnapshot()); originid = replorigin_create(originname); + PopActiveSnapshot(); LockRelationOid(ReplicationOriginRelationId, RowExclusiveLock); replorigin_advance(originid, *origin_startpos, InvalidXLogRecPtr, diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 925dff9cc4..804e5dbb75 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -4604,8 +4604,16 @@ run_apply_worker() walrcv_startstreaming(LogRepWorkerWalRcvConn, &options); StartTransactionCommand(); + + /* + * Updating pg_subscription might involve TOAST table access, so + * ensure we have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + UpdateTwoPhaseState(MySubscription->oid, LOGICALREP_TWOPHASE_STATE_ENABLED); MySubscription->twophasestate = LOGICALREP_TWOPHASE_STATE_ENABLED; + PopActiveSnapshot(); CommitTransactionCommand(); } else @@ -4821,7 +4829,15 @@ DisableSubscriptionAndExit(void) /* Disable the subscription */ StartTransactionCommand(); + + /* + * Updating pg_subscription might involve TOAST table access, so ensure we + * have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + DisableSubscription(MySubscription->oid); + PopActiveSnapshot(); CommitTransactionCommand(); /* Ensure we remove no-longer-useful entry for worker's start time */ @@ -4925,6 +4941,12 @@ clear_subscription_skip_lsn(XLogRecPtr finish_lsn) started_tx = true; } + /* + * Updating pg_subscription might involve TOAST table access, so ensure we + * have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Protect subskiplsn of pg_subscription from being concurrently updated * while clearing it. @@ -4983,6 +5005,8 @@ clear_subscription_skip_lsn(XLogRecPtr finish_lsn) heap_freetuple(tup); table_close(rel, NoLock); + PopActiveSnapshot(); + if (started_tx) CommitTransactionCommand(); } -- 2.39.5 (Apple Git-154)