| From: | Michael Paquier <michael(at)paquier(dot)xyz> |
|---|---|
| To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
| Cc: | 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>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com> |
| Subject: | Re: Handle concurrent drop when doing whole database vacuum |
| Date: | 2026-06-23 23:40:22 |
| Message-ID: | ajsZZlgYYPa37CIz@paquier.xyz |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Jun 23, 2026 at 04:33:11PM -0700, Bharath Rupireddy wrote:
> On Tue, Jun 23, 2026 at 4:14 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>>
>> Anyway, I am wondering if we should aim for simpler. Do we really
>> need the extra ACL check when building a list of relations to consider
>> for a manual VACUUM in the pg_class scan? We are going to re-check
>> the permissions once we vacuum each relation in its own transaction,
>> *after* taking a lock on them, making the ACL check safe. That's the
>> vacuum_open_relation()->vacuum_is_permitted_* flow.
>>
>> On a database with many relations where there is no MAINTAIN privilege
>> for the role running the manual VACUUM, it means more transaction
>> overhead because more transactions would need to be created for each
>> relation whose ACLs need to be rechecked, because we don't filter the
>> relations beforehand with the initial pg_class scan, but that would
>> protect from the concurrent drops entirely by limitting the check to
>> be in *one* path: the one analyzing or vacuuming a single relation.
>
> Spot on. My thinking was along similar lines. I'm concerned that in
> the worst case - where the role running a database-wide vacuum has no
> MAINTAIN privilege at all, or has it on only a small subset of tables
> - this approach will unnecessarily do a bunch of memory allocations,
> start a transaction, get a snapshot, commit the transaction, and
> acquire/release locks for each relation. So, IMHO, -1 for this
> approach. Instead, I would prefer using the pg_class_aclcheck_ext
> check and just emitting a WARNING (like the other skipping messages)
> when the relation is concurrently dropped.
>
> That said, I'm open to hearing from others.
I tend to think that you are worrying too much. If a user has a small
subset of tables, they could just run a VACUUM with a list of tables
instead. That's a tradeoff of code simplicity vs cost, and I'm
finding the simplicity argument quite tempting here.
I'd sure welcome Nathan and Jeff opinions (added now in CC) regarding
this line of thoughts; they worked on MAINTAIN, even if the early
relation ACL check when building a list of relations for a
database-wide manual VACUUM predates that.
--
Michael
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bharath Rupireddy | 2026-06-23 23:53:51 | Re: DDL deparse |
| Previous Message | Tristan Partin | 2026-06-23 23:39:30 | Protect against timestamp underflow in uuidv7(interval) |