| From: | Michael Paquier <michael(at)paquier(dot)xyz> |
|---|---|
| To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
| Cc: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, cca5507 <cca5507(at)qq(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Jeff Davis <pgsql(at)j-davis(dot)com> |
| Subject: | Re: Handle concurrent drop when doing whole database vacuum |
| Date: | 2026-06-30 04:47:41 |
| Message-ID: | akNKbT-481DqyNOP@paquier.xyz |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Jun 29, 2026 at 11:54:44AM -0500, Nathan Bossart wrote:
> I'm mostly concerned about reopening the ability for folks to take strong
> locks on catalogs without the necessary privileges, even if they are only
> briefly held. Commit a556549 fixed a real problem, so I'm nervous about
> partially reverting it. IOW my first reaction is that v1 is the right
> idea, i.e., we should just skip the relation if it disappears. I'm not
> sure we even need to log it.
I have finally spent some time with my head down on this problem.
This would be an issue when a role issues a database-wide VACUUM but
lacks grant access to a table it may look at when opening the
relation.
Simple example, two sessions with this setup:
create role popo login;
create table vacuum_tab (a int); -- owner is my superuser
1) session 1: superuser role
BEGIN;
LOCK vacuum_tab;
2) session 2: user popo
-- Allowed to run, and all relations should be skipped.
VACUUM;
And unfortunately, my intuition and memories were wrong. If we simply
remove the early ACL check when the list of relations is built, the
system-wide VACUUM would block when trying to open the relation
vacuum_tab, meaning that lock attempts would stack, and that's what
a556549d7e6d is all about: the VACUUM should run, and skip all
relations.
Keeping the early ACL check and switching to _ext() would keep the
safeguard in place when running a database-wide VACUUM, and address
the concurrent drop issue. The suggestion of not logging that the
relation is gone while checking its ACL while we don't hold a lock on
the relation feels OK here.
Something that still feels off to me is to blindly use _ext() in
vacuum_is_permitted_for_relation(), where we *may* already hold a lock
on the relation whose ACL is checked. In this case missing a relation
is not fine, so this would make the code more brittle in the
single-relation case under autovacuum or a VACUUM with a list of
relations provided by a user.
--
Michael
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David Steele | 2026-06-30 05:15:19 | Re: Return pg_control from pg_backup_stop(). |
| Previous Message | Feng Wu | 2026-06-30 04:33:34 | Re: [PATCH] Avoid internal error for invalid interval typmods |