Re: Handle concurrent drop when doing whole database vacuum

Hi Michael, Bharath, ChangAo,

I tested v3 locally previously, but after thinking through Michael's
argument

> 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
> > in a556549d7e6d.
>

I agree the architectural direction is better.
Removing the early ACL check from get_all_vacuum_rels() means the syscache
lookup that was racing with concurrent DROP never happens at list
construction
at all. The race altogether vanishes and we don't need to handle the
is_missing.
Also it brings the database-wide VACUUM path in line with VACUUM
<table-list> and autovacuum.

On the partial MAINTAIN case, the per-relation transaction cost is real,
and the rejected vacuum_rel calls would still take ShareUpdateExclusiveLock
briefly, which
could contend with autovacuum on tables the user can't vacuum anyway.
The architectural simplification is durable. Seems like the right tradeoff
as this partial MAINTAIN configuration is not widely used.

On the test front: once the race code path is gone, the injection point
with the spec file isn't strictly needed either.
A simpler test that shows "VACUUM with a concurrent drop successfully
completes without error" should suffice.
Bharath's TAP test suggestion under src/test/modules/test_misc seems like a
good fit now.

Regards,
Surya Poondla