Hi
On 25/03/2026 21:38, Zsolt Parragi wrote:
> Shouldn't the patch also include a tap test to verify that the change
> works / fails without it?
Definitely. I just didn't want to invest much time on tests before
getting feedback on the issue itself.
> + /* Skip temp relations belonging to other sessions */
> + {
> + Oid nsp = get_rel_namespace(index->indrelid);
> +
> + if (!isTempOrTempToastNamespace(nsp) && isAnyTempNamespace(nsp))
> + {
>
> Doesn't this result in several repeated syscache lookups?
>
> There's already a SearchSysCacheExsists1 directly above this, then a
> get_rel_namespace, then an isAnyTempNamespace. While this probably
> isn't performance critical, this should be doable with a single
> SearchSysCache1(RELOID...) and then a few conditions, similarly to the
> else branch below this?
You're right. Although it is not performance critical we can solve it
with a single SearchSysCache1.
PFA v3 with the improved fix (0001) and tests (0002).
Thanks for the review!
Best, JimFrom 6699534c03772b0e5b06680b2e382a36eb108a67 Mon Sep 17 00:00:00 2001
From: Jim Jones <[email protected]>
Date: Wed, 25 Mar 2026 23:14:12 +0100
Subject: [PATCH v3 1/2] Skip other sessions' temp tables in REPACK, CLUSTER,
and VACUUM FULL
get_tables_to_repack() was including other sessions' temporary tables
in the work list, causing REPACK and CLUSTER (without arguments) to
attempt to acquire AccessExclusiveLock on them, potentially blocking
for an extended time. Fix by skipping other-session temp tables early
in get_tables_to_repack(), before they are added to the list. Because
an AccessShareLock has already been acquired per relation at that
point, release it before continuing.
Similarly, get_all_vacuum_rels() suffered from the same problem for
VACUUM FULL. Since no per-relation lock is held during list-building
there, a plain skip suffices.
Author: Jim Jones <[email protected]>
Reviewed-by: Chao Li <[email protected]>
Reviewed-by: Zsolt Parragi <[email protected]>
---
src/backend/commands/cluster.c | 25 ++++++++++++++++++++++++-
src/backend/commands/vacuum.c | 5 +++++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 09066db0956..14c11e8e532 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1679,6 +1679,8 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
{
RelToCluster *rtc;
Form_pg_index index;
+ HeapTuple classtup;
+ Form_pg_class classForm;
MemoryContext oldcxt;
index = (Form_pg_index) GETSTRUCT(tuple);
@@ -1693,11 +1695,24 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
continue;
/* Verify that the table still exists; skip if not */
- if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(index->indrelid)))
+ classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(index->indrelid));
+ if (!HeapTupleIsValid(classtup))
{
UnlockRelationOid(index->indrelid, AccessShareLock);
continue;
}
+ classForm = (Form_pg_class) GETSTRUCT(classtup);
+
+ /* Skip temp relations belonging to other sessions */
+ if (classForm->relpersistence == RELPERSISTENCE_TEMP &&
+ !isTempOrTempToastNamespace(classForm->relnamespace))
+ {
+ ReleaseSysCache(classtup);
+ UnlockRelationOid(index->indrelid, AccessShareLock);
+ continue;
+ }
+
+ ReleaseSysCache(classtup);
/* noisily skip rels which the user can't process */
if (!repack_is_permitted_for_relation(cmd, index->indrelid,
@@ -1753,6 +1768,14 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
continue;
}
+ /* Skip temp relations belonging to other sessions */
+ if (class->relpersistence == RELPERSISTENCE_TEMP &&
+ !isTempOrTempToastNamespace(class->relnamespace))
+ {
+ UnlockRelationOid(class->oid, AccessShareLock);
+ continue;
+ }
+
/* noisily skip rels which the user can't process */
if (!repack_is_permitted_for_relation(cmd, class->oid,
GetUserId()))
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index bce3a2daa24..9b0a5a38a8a 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1062,6 +1062,11 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
classForm->relkind != RELKIND_PARTITIONED_TABLE)
continue;
+ /* Skip temp relations belonging to other sessions */
+ if (classForm->relpersistence == RELPERSISTENCE_TEMP &&
+ !isTempOrTempToastNamespace(classForm->relnamespace))
+ continue;
+
/* check permissions of relation */
if (!vacuum_is_permitted_for_relation(relid, classForm, options))
continue;
--
2.43.0
From 97d1b2023b8c4ac5d92610e9761d112ab68cbe23 Mon Sep 17 00:00:00 2001
From: Jim Jones <[email protected]>
Date: Wed, 25 Mar 2026 23:30:55 +0100
Subject: [PATCH v3 2/2] Test VACUUM FULL, CLUSTER, and REPACK with locked temp
tables
This test creates a background session with a temp table and marks
its index as clustered (making it visible to both the pg_class scan
used by VACUUM FULL and REPACK, and the pg_index scan used by CLUSTER),
then holds ACCESS SHARE LOCK in an open transaction. Each command
runs with lock_timeout = '1ms'. Since lock_timeout only fires when a
backend actually blocks waiting for a lock, 1ms is sufficient.
---
src/test/modules/test_misc/meson.build | 1 +
.../t/011_vacuum_cluster_temp_tables.pl | 65 +++++++++++++++++++
2 files changed, 66 insertions(+)
create mode 100644 src/test/modules/test_misc/t/011_vacuum_cluster_temp_tables.pl
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 6e8db1621a7..d64e8df56bf 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -19,6 +19,7 @@ tests += {
't/008_replslot_single_user.pl',
't/009_log_temp_files.pl',
't/010_index_concurrently_upsert.pl',
+ 't/011_vacuum_cluster_temp_tables.pl',
],
# The injection points are cluster-wide, so disable installcheck
'runningcheck': false,
diff --git a/src/test/modules/test_misc/t/011_vacuum_cluster_temp_tables.pl b/src/test/modules/test_misc/t/011_vacuum_cluster_temp_tables.pl
new file mode 100644
index 00000000000..a612b4d6361
--- /dev/null
+++ b/src/test/modules/test_misc/t/011_vacuum_cluster_temp_tables.pl
@@ -0,0 +1,65 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+#
+# Verify that no-argument VACUUM FULL, CLUSTER, and REPACK skip temporary
+# tables belonging to other sessions.
+#
+# A background session creates a temp table and marks its index as clustered —
+# making it visible to both the pg_class scan (VACUUM FULL, REPACK) and the
+# pg_index scan (CLUSTER) — then holds ACCESS SHARE LOCK in an open transaction.
+# Each command runs with lock_timeout = '1ms'. Since lock_timeout only
+# fires when a backend actually blocks waiting for a lock, 1ms is sufficient.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('vacuum_cluster_temp');
+$node->init;
+$node->start;
+
+# Session 1: build the temp table and hold a conflicting lock.
+my $psql1 = $node->background_psql('postgres');
+
+$psql1->query_safe(
+ q{CREATE TEMP TABLE temp_repack_test (val int);
+ INSERT INTO temp_repack_test VALUES (1);
+ CREATE INDEX temp_repack_idx ON temp_repack_test (val);
+ CLUSTER temp_repack_test USING temp_repack_idx;});
+
+$psql1->query_safe(q{BEGIN});
+$psql1->query_safe(q{LOCK TABLE temp_repack_test IN ACCESS SHARE MODE});
+
+my ($stdout, $stderr, $ret);
+
+# VACUUM FULL — pg_class scan path.
+$ret = $node->psql(
+ 'postgres',
+ "SET lock_timeout = '1ms'; VACUUM FULL;",
+ stdout => \$stdout,
+ stderr => \$stderr);
+is($ret, 0,
+ 'VACUUM FULL completes without blocking on another session temp table');
+
+# CLUSTER — pg_index scan path (indisclustered entries).
+$ret = $node->psql(
+ 'postgres',
+ "SET lock_timeout = '1ms'; CLUSTER;",
+ stdout => \$stdout,
+ stderr => \$stderr);
+is($ret, 0,
+ 'CLUSTER completes without blocking on another session temp table');
+
+# REPACK — pg_class scan path.
+$ret = $node->psql(
+ 'postgres',
+ "SET lock_timeout = '1ms'; REPACK;",
+ stdout => \$stdout,
+ stderr => \$stderr);
+is($ret, 0,
+ 'REPACK completes without blocking on another session temp table');
+
+$psql1->quit;
+
+done_testing();
--
2.43.0
