From: Nathan Bossart <nathandbossart@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alexander Lakhin <exclusion@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Jonathan S. Katz <jkatz@postgresql.org>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Large expressions in indexes can't be stored (non-TOASTable)
Date: Wed, 30 Oct 2024 15:54:32 -0500
Message-ID: <ZyKdCEaUB2lB1rG8@nathan> (raw)
In-Reply-To: <ZxGZB2d-RttHwmlt@paquier.xyz>
References: <ZvI-WXcaFXuj1Dvp@paquier.xyz>
<ZvMSUPOqUU-VNADN@nathan>
<ZvOMBhlb5zrBCG5p@paquier.xyz>
<ZwV_DBQa8Qw6kKY2@nathan>
<Zw14zoBCg-zGu1mi@nathan>
<Zw8E73_nCk0DbO18@paquier.xyz>
<Zw8U0YU_fu2pcXs6@nathan>
<Zw8V0ClvZbUi2QQl@paquier.xyz>
<Zw_hxaDoC3TzQYjj@nathan>
<ZxGZB2d-RttHwmlt@paquier.xyz>
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
Attachments:v5-0001-Ensure-we-have-a-snapshot-when-updating-va.patch.master (text/plain, 6.8K) download
From 58eefc6c2f90ac7d1f23f5b5f3052042cf0e75d9 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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)
v5-0001-Ensure-we-have-a-snapshot-when-updating-vario.patch.v12 (text/plain, 2.4K) download
From cd8058dd2792f99a390cfb4794fc77921ececc51 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@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.cindex 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.cindex 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)
v5-0001-Ensure-we-have-a-snapshot-when-updating-vario.patch.v13 (text/plain, 2.4K) download
From 61c70510702c50e284ab5ed4db6971e3d5a7e836 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@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.cindex 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.cindex 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)
v5-0001-Ensure-we-have-a-snapshot-when-updating-vario.patch.v14 (text/plain, 4.7K) download
From 51d43367aef06d43c14fab9985a55fd4dbb67bc2 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@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.cindex 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.cindex 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.cindex 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.cindex 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)
v5-0001-Ensure-we-have-a-snapshot-when-updating-vario.patch.v15 (text/plain, 6.6K) download
From b827d58167498f0a5dfcf95e84fe297bdeb7b1ad Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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)
v5-0001-Ensure-we-have-a-snapshot-when-updating-vario.patch.v16 (text/plain, 7.2K) download
From 132fe7048627a6d59e7b4310b03425ce8119893a Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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)
v5-0001-Ensure-we-have-a-snapshot-when-updating-vario.patch.v17 (text/plain, 7.2K) download
From ef3a3a3aeab379540f894762e798caae3bbc29a7 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, michael@paquier.xyz, exclusion@gmail.com, tgl@sss.pgh.pa.us, jkatz@postgresql.org, pgsql-hackers@lists.postgresql.org
Subject: Re: Large expressions in indexes can't be stored (non-TOASTable)
In-Reply-To: <ZyKdCEaUB2lB1rG8@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox