On Fri, Oct 18, 2024 at 08:08:55AM +0900, Michael Paquier wrote: > 0002 and 0003 look sane enough to me as shaped. I'd need to spend a > few more hours on 0001 if I were to do a second round on it, but you > don't really need a second opinion, do you? :D
I'll manage. 0001 was a doozy to back-patch, and this obviously isn't a terribly pressing issue, so I plan to wait until after the November minor release to apply this. I'm having second thoughts on 0002, so I'll probably leave that one uncommitted, but IMHO we definitely need 0003 to prevent this issue from reoccurring. -- nathan
>From 58eefc6c2f90ac7d1f23f5b5f3052042cf0e75d9 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Wed, 30 Oct 2024 15:42:29 -0500 Subject: [PATCH v5 1/1] 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: 12 --- src/backend/commands/indexcmds.c | 2 +- src/backend/commands/tablecmds.c | 8 +++++++ src/backend/postmaster/autovacuum.c | 7 ++++++ src/backend/replication/logical/tablesync.c | 21 ++++++++++++++++++ src/backend/replication/logical/worker.c | 24 +++++++++++++++++++++ 5 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 2f652463e3..fd86f28a35 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -4230,7 +4230,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein false); /* - * Updating pg_index might involve TOAST table access, so ensure we + * Swapping the indexes might involve TOAST table access, so ensure we * have a valid snapshot. */ PushActiveSnapshot(GetTransactionSnapshot()); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 4345b96de5..f1649419d6 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -19383,9 +19383,17 @@ 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. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* Do the final part of detaching */ DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid); + PopActiveSnapshot(); + ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); /* keep our lock until commit */ diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index dc3cf87aba..e84285b644 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2209,6 +2209,12 @@ do_autovacuum(void) get_namespace_name(classForm->relnamespace), NameStr(classForm->relname)))); + /* + * Deletion might involve TOAST table access, so ensure we have a + * valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + object.classId = RelationRelationId; object.objectId = relid; object.objectSubId = 0; @@ -2221,6 +2227,7 @@ do_autovacuum(void) * To commit the deletion, end current transaction and start a new * one. Note this also releases the locks we took. */ + PopActiveSnapshot(); CommitTransactionCommand(); StartTransactionCommand(); diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index 118503fcb7..57e1eede41 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. */ @@ -1462,8 +1478,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)
>From cd8058dd2792f99a390cfb4794fc77921ececc51 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Wed, 30 Oct 2024 15:42:29 -0500 Subject: [PATCH v5 1/1] 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: 12 --- src/backend/commands/indexcmds.c | 8 ++++++++ src/backend/postmaster/autovacuum.c | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index e3c61cd907..3168442ea4 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -3477,12 +3477,20 @@ ReindexRelationConcurrently(Oid relationOid, int options) get_rel_namespace(heapId), false); + /* + * Swapping the indexes might involve TOAST table access, so ensure we + * have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Swap old index with the new one. This also marks the new one as * valid and the old one as not valid. */ index_concurrently_swap(newIndexId, oldIndexId, oldName); + PopActiveSnapshot(); + /* * Invalidate the relcache for the table, so that after this commit * all sessions will refresh any cached plans that might reference the diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index dd5050dad8..a6695b74ff 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2296,6 +2296,12 @@ do_autovacuum(void) get_namespace_name(classForm->relnamespace), NameStr(classForm->relname)))); + /* + * Deletion might involve TOAST table access, so ensure we have a + * valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + object.classId = RelationRelationId; object.objectId = relid; object.objectSubId = 0; @@ -2308,6 +2314,7 @@ do_autovacuum(void) * To commit the deletion, end current transaction and start a new * one. Note this also releases the locks we took. */ + PopActiveSnapshot(); CommitTransactionCommand(); StartTransactionCommand(); -- 2.39.5 (Apple Git-154)
>From 61c70510702c50e284ab5ed4db6971e3d5a7e836 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Wed, 30 Oct 2024 15:42:29 -0500 Subject: [PATCH v5 1/1] 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: 12 --- src/backend/commands/indexcmds.c | 8 ++++++++ src/backend/postmaster/autovacuum.c | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 13fe116f71..9accf86503 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -3512,12 +3512,20 @@ ReindexRelationConcurrently(Oid relationOid, int options) get_rel_namespace(heapId), false); + /* + * Swapping the indexes might involve TOAST table access, so ensure we + * have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Swap old index with the new one. This also marks the new one as * valid and the old one as not valid. */ index_concurrently_swap(newIndexId, oldIndexId, oldName); + PopActiveSnapshot(); + /* * Invalidate the relcache for the table, so that after this commit * all sessions will refresh any cached plans that might reference the diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 94828de9fe..a229741448 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2292,6 +2292,12 @@ do_autovacuum(void) get_namespace_name(classForm->relnamespace), NameStr(classForm->relname)))); + /* + * Deletion might involve TOAST table access, so ensure we have a + * valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + object.classId = RelationRelationId; object.objectId = relid; object.objectSubId = 0; @@ -2304,6 +2310,7 @@ do_autovacuum(void) * To commit the deletion, end current transaction and start a new * one. Note this also releases the locks we took. */ + PopActiveSnapshot(); CommitTransactionCommand(); StartTransactionCommand(); -- 2.39.5 (Apple Git-154)
>From 51d43367aef06d43c14fab9985a55fd4dbb67bc2 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Wed, 30 Oct 2024 15:42:29 -0500 Subject: [PATCH v5 1/1] 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: 12 --- src/backend/commands/indexcmds.c | 8 ++++++++ src/backend/commands/tablecmds.c | 8 ++++++++ src/backend/postmaster/autovacuum.c | 7 +++++++ src/backend/replication/logical/tablesync.c | 13 +++++++++++++ 4 files changed, 36 insertions(+) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 99c536891a..506039ba5a 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -3998,12 +3998,20 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params) get_rel_namespace(oldidx->tableId), false); + /* + * Swapping the indexes might involve TOAST table access, so ensure we + * have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Swap old index with the new one. This also marks the new one as * valid and the old one as not valid. */ index_concurrently_swap(newidx->indexId, oldidx->indexId, oldName); + PopActiveSnapshot(); + /* * Invalidate the relcache for the table, so that after this commit * all sessions will refresh any cached plans that might reference the diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 6dd4e88d48..d3ddb0c873 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -18200,9 +18200,17 @@ 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. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* Do the final part of detaching */ DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid); + PopActiveSnapshot(); + ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); /* keep our lock until commit */ diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 192837bfa3..de84c67ad9 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2319,6 +2319,12 @@ do_autovacuum(void) get_namespace_name(classForm->relnamespace), NameStr(classForm->relname)))); + /* + * Deletion might involve TOAST table access, so ensure we have a + * valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + object.classId = RelationRelationId; object.objectId = relid; object.objectSubId = 0; @@ -2331,6 +2337,7 @@ do_autovacuum(void) * To commit the deletion, end current transaction and start a new * one. Note this also releases the locks we took. */ + PopActiveSnapshot(); CommitTransactionCommand(); StartTransactionCommand(); diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index 26b71dee67..852ce4f41f 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -451,6 +451,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. * @@ -470,6 +476,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. */ @@ -1092,8 +1100,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, -- 2.39.5 (Apple Git-154)
>From b827d58167498f0a5dfcf95e84fe297bdeb7b1ad Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Wed, 30 Oct 2024 15:42:29 -0500 Subject: [PATCH v5 1/1] 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: 12 --- src/backend/commands/indexcmds.c | 8 +++++++ src/backend/commands/tablecmds.c | 8 +++++++ src/backend/postmaster/autovacuum.c | 7 ++++++ src/backend/replication/logical/tablesync.c | 13 +++++++++++ src/backend/replication/logical/worker.c | 24 +++++++++++++++++++++ 5 files changed, 60 insertions(+) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 157455c52b..99ba76fca3 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -3986,12 +3986,20 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params) get_rel_namespace(oldidx->tableId), false); + /* + * Swapping the indexes might involve TOAST table access, so ensure we + * have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Swap old index with the new one. This also marks the new one as * valid and the old one as not valid. */ index_concurrently_swap(newidx->indexId, oldidx->indexId, oldName); + PopActiveSnapshot(); + /* * Invalidate the relcache for the table, so that after this commit * all sessions will refresh any cached plans that might reference the diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1402755c36..0b80c64390 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -18769,9 +18769,17 @@ 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. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* Do the final part of detaching */ DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid); + PopActiveSnapshot(); + ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); /* keep our lock until commit */ diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 356678b030..f93822cb5b 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2298,6 +2298,12 @@ do_autovacuum(void) get_namespace_name(classForm->relnamespace), NameStr(classForm->relname)))); + /* + * Deletion might involve TOAST table access, so ensure we have a + * valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + object.classId = RelationRelationId; object.objectId = relid; object.objectSubId = 0; @@ -2310,6 +2316,7 @@ do_autovacuum(void) * To commit the deletion, end current transaction and start a new * one. Note this also releases the locks we took. */ + PopActiveSnapshot(); CommitTransactionCommand(); StartTransactionCommand(); diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index e6159acba0..2314155338 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -458,6 +458,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. * @@ -477,6 +483,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. */ @@ -1387,8 +1395,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 dcc3fdf6c7..155bf7c44e 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -3794,8 +3794,16 @@ ApplyWorkerMain(Datum main_arg) 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 @@ -3848,7 +3856,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(); /* Notify the subscription has been disabled and exit */ @@ -3939,6 +3955,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. @@ -3997,6 +4019,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)
>From 132fe7048627a6d59e7b4310b03425ce8119893a Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Wed, 30 Oct 2024 15:42:29 -0500 Subject: [PATCH v5 1/1] 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: 12 --- src/backend/commands/indexcmds.c | 8 +++++++ src/backend/commands/tablecmds.c | 8 +++++++ src/backend/postmaster/autovacuum.c | 7 ++++++ src/backend/replication/logical/tablesync.c | 21 ++++++++++++++++++ src/backend/replication/logical/worker.c | 24 +++++++++++++++++++++ 5 files changed, 68 insertions(+) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 9afd10ec4e..f1d4825e33 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -4057,12 +4057,20 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params) get_rel_namespace(oldidx->tableId), false); + /* + * Swapping the indexes might involve TOAST table access, so ensure we + * have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Swap old index with the new one. This also marks the new one as * valid and the old one as not valid. */ index_concurrently_swap(newidx->indexId, oldidx->indexId, oldName); + PopActiveSnapshot(); + /* * Invalidate the relcache for the table, so that after this commit * all sessions will refresh any cached plans that might reference the diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 0e47f1a457..65103700e5 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -18748,9 +18748,17 @@ 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. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* Do the final part of detaching */ DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid); + PopActiveSnapshot(); + ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); /* keep our lock until commit */ diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 7dd9345c61..669e0fb004 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2355,6 +2355,12 @@ do_autovacuum(void) get_namespace_name(classForm->relnamespace), NameStr(classForm->relname)))); + /* + * Deletion might involve TOAST table access, so ensure we have a + * valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + object.classId = RelationRelationId; object.objectId = relid; object.objectSubId = 0; @@ -2367,6 +2373,7 @@ do_autovacuum(void) * To commit the deletion, end current transaction and start a new * one. Note this also releases the locks we took. */ + PopActiveSnapshot(); CommitTransactionCommand(); StartTransactionCommand(); diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index 7d4ee48206..faa467df7b 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -376,6 +376,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. * @@ -386,6 +392,8 @@ process_syncing_tables_for_sync(XLogRecPtr current_lsn) */ replorigin_drop_by_name(originname, true, false); + PopActiveSnapshot(); + finish_sync_worker(); } else @@ -482,6 +490,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. * @@ -499,6 +513,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. */ @@ -1442,8 +1458,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 18f86c73bd..c37eac9e72 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -4715,8 +4715,16 @@ ApplyWorkerMain(Datum main_arg) 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 @@ -4769,7 +4777,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 */ @@ -4873,6 +4889,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. @@ -4931,6 +4953,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)
>From ef3a3a3aeab379540f894762e798caae3bbc29a7 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Wed, 30 Oct 2024 15:42:29 -0500 Subject: [PATCH v5 1/1] 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: 12 --- src/backend/commands/indexcmds.c | 8 +++++++ src/backend/commands/tablecmds.c | 8 +++++++ src/backend/postmaster/autovacuum.c | 7 ++++++ src/backend/replication/logical/tablesync.c | 21 ++++++++++++++++++ src/backend/replication/logical/worker.c | 24 +++++++++++++++++++++ 5 files changed, 68 insertions(+) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 3d8df046da..654f9262b1 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -4087,12 +4087,20 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein get_rel_namespace(oldidx->tableId), false); + /* + * Swapping the indexes might involve TOAST table access, so ensure we + * have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Swap old index with the new one. This also marks the new one as * valid and the old one as not valid. */ index_concurrently_swap(newidx->indexId, oldidx->indexId, oldName); + PopActiveSnapshot(); + /* * Invalidate the relcache for the table, so that after this commit * all sessions will refresh any cached plans that might reference the diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 36717ffcb0..e94e88652e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -19227,9 +19227,17 @@ 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. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* Do the final part of detaching */ DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid); + PopActiveSnapshot(); + ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); /* keep our lock until commit */ diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 8f27026d19..a27ae14b72 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2201,6 +2201,12 @@ do_autovacuum(void) get_namespace_name(classForm->relnamespace), NameStr(classForm->relname)))); + /* + * Deletion might involve TOAST table access, so ensure we have a + * valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + object.classId = RelationRelationId; object.objectId = relid; object.objectSubId = 0; @@ -2213,6 +2219,7 @@ do_autovacuum(void) * To commit the deletion, end current transaction and start a new * one. Note this also releases the locks we took. */ + PopActiveSnapshot(); CommitTransactionCommand(); StartTransactionCommand(); diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index b00267f042..e1e5d80997 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. */ @@ -1454,8 +1470,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 d091a1dd27..95b680fde4 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -4529,8 +4529,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 @@ -4746,7 +4754,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 */ @@ -4850,6 +4866,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. @@ -4908,6 +4930,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)
