Re: Vacuum now uses AccessShareLock for analyze

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> The code will now vacuum all requested relations. It will then analyze
> each relation. This way, all the exclusive vacuum work is done first,
> then analyze can continue with shared locks.

I agree with Marc: it'd make more sense to do it one table at a time,
ie,
get exclusive lock on table A
vacuum table A
commit, release lock
get shared lock on table A
gather stats for table A
commit, release lock
repeat sequence for table B
etc

> The code is much clearer with that functionality split into separate
> functions.

Wouldn't surprise me.

> How separate do people want vacuum and analyze? Analyze currently does
> not record the number of tuples and pages, because vacuum does that. Do
> people want analyze as a separate command and in a separate file?

We definitely want a separate command that can invoke just the analyze
part. I'd guess something like "ANALYZE [ VERBOSE ] optional-table-name
(optional-list-of-columns)" pretty much like VACUUM.

I would be inclined to move the code out to a new file, just because
vacuum.c is so darn big, but that's purely a code-beautification issue.

On the number of tuples/pages issue, I'd suggest removing that function
from plain vacuum and make the analyze part do it instead. It's always
made me uncomfortable that vacuum needs to update system relations while
it's holding an exclusive lock on the table-being-vacuumed (which might
be another system catalog, or even pg_class itself). It works, more or
less, but that update-tuple-in-place code is awfully ugly and
fragile-looking. I'm also worried that there could be deadlock
scenarios between concurrent vacuums (eg, one guy working on pg_class,
another on pg_statistic, both need to get in and update the other guy's
table. Oops. That particular problem should be gone with your changes,
but maybe there are still problems just from the need to update
pg_class).

regards, tom lane