Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1t6FiG-00BhYl-97 for pgsql-hackers@arkaria.postgresql.org; Wed, 30 Oct 2024 20:54:44 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1t6FiE-00COBc-Jg for pgsql-hackers@arkaria.postgresql.org; Wed, 30 Oct 2024 20:54:42 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1t6FiE-00COBR-4C for pgsql-hackers@lists.postgresql.org; Wed, 30 Oct 2024 20:54:42 +0000 Received: from mail-il1-x12a.google.com ([2607:f8b0:4864:20::12a]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1t6Fi9-003sfg-6t for pgsql-hackers@lists.postgresql.org; Wed, 30 Oct 2024 20:54:41 +0000 Received: by mail-il1-x12a.google.com with SMTP id e9e14a558f8ab-3a4e5e574b6so1074875ab.0 for ; Wed, 30 Oct 2024 13:54:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730321675; x=1730926475; darn=lists.postgresql.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=ZtuGT6eI9do/Ki/LKUxFPnzpb9W/HqFbOymNIilLWdw=; b=Rk6nfck3UQoJAsohNQVRqjrdFWRTtRU44AmdRtEB30alfY4fAl8oLjsa11kyfqIOBA N8E73MBgc0JDbtJ33ZqbFuCF9FK8dQXz0xVoyly22yDOhWnYz59mLKO0loKFkwy4merJ 7cd0B0a4wS5fPka0eyn+4spxwS1RbBWpsIHwIMWZ2aX17nLAAaxc2nDoLXalpFIhDOIy SPYBeIPSotTCKMGxULoYZx8cQ8ci0pxcQuZa0zSIT4Wg7mM3+UxAG9SnnzxsTlF2U6MR A41MkUz9xNp3H8p4VKUYbcPpxy6jmCJ7sHC7m8Dp9QL+TeCGO1Nfkfzh3xCMM7wTO0Oa kQfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730321675; x=1730926475; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ZtuGT6eI9do/Ki/LKUxFPnzpb9W/HqFbOymNIilLWdw=; b=neWJ0bEGlvnSdmDJcUQNAsJ1lvbXWHBwgiyNhT+ZtTfcnRv1PIdyzrwkH0jQUXC549 f1CeJOcbgqUcJxY0f/3FaX+cDsis5lfAVPTvuu3VoSP4dfMXBCXqksjfiMacrFWJNrSg n8djy0cgCnbtgkfiKBk79EQjGICtO842uUY0PBjoBqep/baH3WZGw62CY+Z+sNKs5SaU JmeR7AhqYAH0SiCt/Y6H73rBe9aeQg5AowLJpqUxXmHLHFXO4EG0c+Ad78gif/nAs+P0 C3LjG0b4Dxfh0BwPCML8OStqya5x2OzXeQwIA5sgvJNWlS70rm6vVm9Ypb6lO7PWxs9y LR5A== X-Forwarded-Encrypted: i=1; AJvYcCV5KksDfwMjNka1wGPCNHPJ/AbVAnRYfpvILzFNfEJKMDNlDb9mb0/l8i1q8RqLOMt0zYAmdDVTvigxWJU5@lists.postgresql.org X-Gm-Message-State: AOJu0YxB3TdpDy/ZMYWptnH1UURqxgOwqHNurlDtmN49jkWAA73RIoDx WC6BR4lix68vILLdnxIGqWHzDoAw2YsHgnlejfWn6Y+rRchPV9JG X-Google-Smtp-Source: AGHT+IGH4zeT/8l+7tivCF1QEO5IuPkI5Kzs+NV/Y4TP1hZQ8+VyKexc+x8vG/i27pH/NkzSLEpjnA== X-Received: by 2002:a05:6e02:54c:b0:3a3:637f:1012 with SMTP id e9e14a558f8ab-3a6a9563d46mr313585ab.12.1730321675158; Wed, 30 Oct 2024 13:54:35 -0700 (PDT) Received: from nathan (162-195-168-172.lightspeed.stlsmo.sbcglobal.net. [162.195.168.172]) by smtp.gmail.com with ESMTPSA id 8926c6da1cb9f-4dc725eb460sm2986663173.30.2024.10.30.13.54.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Oct 2024 13:54:34 -0700 (PDT) Date: Wed, 30 Oct 2024 15:54:32 -0500 From: Nathan Bossart To: Michael Paquier Cc: Alexander Lakhin , Tom Lane , "Jonathan S. Katz" , PostgreSQL Hackers Subject: Re: Large expressions in indexes can't be stored (non-TOASTable) Message-ID: References: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="tY2erLxPspQxwzxG" Content-Disposition: inline In-Reply-To: List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --tY2erLxPspQxwzxG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 --tY2erLxPspQxwzxG Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="v5-0001-Ensure-we-have-a-snapshot-when-updating-va.patch.master" From 58eefc6c2f90ac7d1f23f5b5f3052042cf0e75d9 Mon Sep 17 00:00:00 2001 From: Nathan Bossart 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) --tY2erLxPspQxwzxG Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="v5-0001-Ensure-we-have-a-snapshot-when-updating-vario.patch.v12" From cd8058dd2792f99a390cfb4794fc77921ececc51 Mon Sep 17 00:00:00 2001 From: Nathan Bossart 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) --tY2erLxPspQxwzxG Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="v5-0001-Ensure-we-have-a-snapshot-when-updating-vario.patch.v13" From 61c70510702c50e284ab5ed4db6971e3d5a7e836 Mon Sep 17 00:00:00 2001 From: Nathan Bossart 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) --tY2erLxPspQxwzxG Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="v5-0001-Ensure-we-have-a-snapshot-when-updating-vario.patch.v14" From 51d43367aef06d43c14fab9985a55fd4dbb67bc2 Mon Sep 17 00:00:00 2001 From: Nathan Bossart 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) --tY2erLxPspQxwzxG Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="v5-0001-Ensure-we-have-a-snapshot-when-updating-vario.patch.v15" From b827d58167498f0a5dfcf95e84fe297bdeb7b1ad Mon Sep 17 00:00:00 2001 From: Nathan Bossart 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) --tY2erLxPspQxwzxG Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="v5-0001-Ensure-we-have-a-snapshot-when-updating-vario.patch.v16" From 132fe7048627a6d59e7b4310b03425ce8119893a Mon Sep 17 00:00:00 2001 From: Nathan Bossart 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) --tY2erLxPspQxwzxG Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="v5-0001-Ensure-we-have-a-snapshot-when-updating-vario.patch.v17" From ef3a3a3aeab379540f894762e798caae3bbc29a7 Mon Sep 17 00:00:00 2001 From: Nathan Bossart 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) --tY2erLxPspQxwzxG--