Re: [HACKERS] Autovacuum improvements

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Here it is.

I'd drop the InitProcess API change, which touches many more places than
you really need, and just have InitProcess check IsAutoVacuumProcess(),
which should be valid by the time control gets to it. This is more like
the way that InitPostgres handles it, anyway.

> Note that I used the same DatabaseHasActiveBackends() function to do the
> kill. I had first added a different one to kill autovacuum, but then
> noticed that this one has no callers that don't want the side effect, so
> I merged them. It seems a bit ugly to me to have a function named like
> this and still have the side effect, but on the other hand it's quite
> useless to have a version without the side effect that will never get
> called.

Agreed; maybe change the name to something that sounds less like a
side-effect-free function?

> Another point to make is that it only kills autovacuum, and only if no
> other process is found. So if there are two processes and autovacuum is
> one of them, it will be allowed to continue.

What if there are two autovac processes, which seems likely to be
possible soon enough?

> I feel that changing the DROP DATABASE behavior with respect to killing
> other backends is beyond the scope of this patch. It seems easy enough
> to do if somebody feels so inclined.

I don't think the idea of killing non-autovac processes will fly.
Waiting for them, on the other hand, might.

> + /* good, there's only an autovacuum -- kill it */
> + kill(autovacPid, SIGINT);
> + LWLockRelease(ProcArrayLock);

Please release the LWLock *before* executing a kernel call ...

regards, tom lane