Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables - Mailing list pgsql-hackers
| From | Chao Li |
|---|---|
| Subject | Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables |
| Date | |
| Msg-id | 97B93CF6-109D-46B4-AD50-8908DD4BE6E4@gmail.com Whole thread Raw |
| In response to | VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables (Jim Jones <jim.jones@uni-muenster.de>) |
| Responses |
Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
|
| List | pgsql-hackers |
> On Mar 24, 2026, at 23:35, Jim Jones <jim.jones@uni-muenster.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/
pgsql-hackers by date: