| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Antonin Houska <ah(at)cybertec(dot)at> |
| Subject: | Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables |
| Date: | 2026-03-25 01:55:29 |
| Message-ID: | 97B93CF6-109D-46B4-AD50-8908DD4BE6E4@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Mar 24, 2026, at 23:35, Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> wrote:
>
> Hi,
>
> While testing another patch [1], I noticed that REPACK is blocked when a
> temporary table is locked in another session. It also turns out that the
> same behaviour occurs with VACUUM FULL and CLUSTER:
>
> == session 1 ==
>
> $ psql postgres
> psql (19devel)
> Type "help" for help.
>
> postgres=# CREATE TEMPORARY TABLE tmp (id int);
> CREATE TABLE
> postgres=# BEGIN;
> LOCK TABLE tmp IN SHARE MODE;
> BEGIN
> LOCK TABLE
> postgres=*#
>
> == session 2 ==
>
> $ psql postgres
> psql (19devel)
> Type "help" for help.
>
> postgres=# REPACK;
> ^CCancel request sent
> ERROR: canceling statement due to user request
> CONTEXT: waiting for AccessExclusiveLock on relation 38458 of database 5
> postgres=# VACUUM FULL;
> ^CCancel request sent
> ERROR: canceling statement due to user request
> CONTEXT: waiting for AccessExclusiveLock on relation 38458 of database 5
>
> Skipping temporary relations in get_tables_to_repack() and
> get_all_vacuum_rels() before they're appended to the list seems to do
> the trick -- see attached draft.
>
> I can reproduce the same behaviour with CLUSTER and VACUUM FULL in
> PG14-PG18. I took a quick look at the code in PG17 and PG18 and the fix
> appears to be straightforward, but before I start working on it, I'd
> like to hear your thoughts. Is it worth the effort?
>
> Best, Jim
>
> 1 - https://www.postgresql.org/message-id/13637.1774342137%40localhost<v1-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch>
I think skipping temp tables of another session is reasonable, because anyway they are not accessible from the current session, though visible via pg_class.
Looking at the patch:
```
+ /* Skip temp relations belonging to other sessions */
+ if (class->relpersistence == RELPERSISTENCE_TEMP &&
+ isOtherTempNamespace(class->relnamespace))
```
It uses isOtherTempNamespace(), but I noticed that the header comment of the function says:
```
* isOtherTempNamespace - is the given namespace some other backend's
* temporary-table namespace (including temporary-toast-table namespaces)?
*
* Note: for most purposes in the C code, this function is obsolete. Use
* RELATION_IS_OTHER_TEMP() instead to detect non-local temp relations.
```
Then looking at RELATION_IS_OTHER_TEMP():
```
#define RELATION_IS_OTHER_TEMP(relation) \
((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP && \
!(relation)->rd_islocaltemp)
```
It takes a relation as parameter and check relation->rd_islocaltemp, however in the context of this patch, we have only Form_pg_class.
Then checking how rd_islocaltemp is set:
```
case RELPERSISTENCE_TEMP:
if (isTempOrTempToastNamespace(relation->rd_rel->relnamespace))
{
relation->rd_backend = ProcNumberForTempRelations();
relation->rd_islocaltemp = true;
}
else
{
/*
* If it's a temp table, but not one of ours, we have to use
* the slow, grotty method to figure out the owning backend.
*
* Note: it's possible that rd_backend gets set to
* MyProcNumber here, in case we are looking at a pg_class
* entry left over from a crashed backend that coincidentally
* had the same ProcNumber we're using. We should *not*
* consider such a table to be "ours"; this is why we need the
* separate rd_islocaltemp flag. The pg_class entry will get
* flushed if/when we clean out the corresponding temp table
* namespace in preparation for using it.
*/
relation->rd_backend =
GetTempNamespaceProcNumber(relation->rd_rel->relnamespace);
Assert(relation->rd_backend != INVALID_PROC_NUMBER);
relation->rd_islocaltemp = false;
}
break;
```
It uses isTempOrTempToastNamespace(relation->rd_rel->relnamespace) to decide relation->rd_islocaltemp.
So, I think this patch should also use "!isTempOrTempToastNamespace(classForm->relnamespace)" instead of isOtherTempNamespace(class->relnamespace). I tried that locally, and it works for me.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-03-25 01:55:58 | Re: [PATCH] Auto vacuum should still run when clock is set back |
| Previous Message | Henson Choi | 2026-03-25 01:53:55 | Re: Minor refactor of the code in ExecScanExtended() |