Re: Autovacuum launcher

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Here's the autovacuum launcher patch I'm considering for inclusion.

This part is very seriously broken:

diff -c -p -r1.33 lwlock.h
*** src/include/storage/lwlock.h 5 Jan 2007 22:19:58 -0000 1.33
--- src/include/storage/lwlock.h 13 Feb 2007 16:58:41 -0000
*************** typedef enum LWLockId
*** 62,67 ****
--- 62,68 ----
BtreeVacuumLock,
AddinShmemInitLock,
FirstBufMappingLock,
+ AutovacuumLock,
FirstLockMgrLock = FirstBufMappingLock + NUM_BUFFER_PARTITIONS,

/* must be last except for MaxDynamicLWLock: */

I'm surprised it got through your testing at all, with the autovacuum
lock conflicting with bufmgr.

Making InitPostgres's call API vary depending on
IsAutoVacuumWorkerProcess seems really ugly, and unnecessary.
Why not just test for dbname == NULL or some other convention
expressed by the arguments themselves?

> One thing I noticed is that when autovac is off, it may take up to 60
> seconds to process a signal passed down via SendPostmasterSignal().

That's because if the signal arrives while postmaster has signals
blocked, it'll be serviced at PG_SETMASK(&UnBlockSig), and then (if no
connection requests are arriving) the select() timeout delay will elapse
before the main loop notices the flag. You probably want to reconsider
the decision to not have sigusr1_handler() launch the child immediately.
The current coding is the way it is because an autovac child can itself
send the signal and we want that to be interpreted as "start another one
after the current one finishes". In the launcher-and-workers world,
though, I'm not convinced we need any such behavior.

One problem I see with this is you've removed the code that sent SIGINT
to an active autovac process during "smart shutdown". This seems a bad
idea because now shutdown response will be limited by the slowest
autovac, and that will tempt DBAs with itchy trigger fingers to do
something stupid. It'd be a good idea to extend the Backend struct
to mark which backends are autovacs, so that you can preserve the
behavior of sending them SIGINTs at shutdown.

regards, tom lane