Re: Handle concurrent drop when doing whole database vacuum

Hi,

On Fri, Jun 26, 2026 at 2:36 AM cca5507 <cca5507(at)qq(dot)com> wrote:
>
> Hi,
>
> > > FWIW, even after sleeping on it, I don't think that this patch is
> > > going in the right direction. I am pretty sure that we should just
> > > lift the ACL check in get_all_vacuum_rels() and always require
> > > vacuum_is_permitted_for_relation() to have the relation locked when
> > > called. This way we would rely on one single code path for the ACL
> > > check, even if it means holding a reference of a relation OID for
> > > something to be processsed later. So I would revisit the choice made
> >
> > FWIW, I agree with this direction.
>
> This direction is also OK for me. We might want to fix get_tables_to_repack()
> together?

We want to get the vacuum behavior consistent across all three modes -
database-wide vacuum, vacuum <tables-list>, and autovacuum - as far as
the permissions check is concerned. That is, do the permissions check
after acquiring the table-level lock. This also solves the ERRORs
reported due to concurrent table drops during the permissions check,
which happen because pg_class_aclcheck is used instead of
pg_class_aclcheck_ext with is_missing.

I had a quick look at the repack code and it seems like it can also
run into the same problem because repack_is_permitted_for_relation
uses the same pg_class_aclcheck while building the tables list without
holding locks, and later it rechecks permissions after the table locks
in cluster_rel_recheck anyway. A simple fix there would be to just use
pg_class_aclcheck_ext in repack_is_permitted_for_relation. I recommend
discussing this in a separate thread CC-ing the repack authors to get
agreement.

Do you mind sharing the vacuum-related fix where we have agreement in
this thread? Thank you!

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com