Michael Paquier wrote: > So, attached are two patches: > - 0001 enables SIGHUP tracking in do_autovacuum(), which is checked > before processing one table. I reused avl_sighup_handler for the > worker, renaming it av_sighup_handler.. > - 0002 is the patch to add log_autovacuum_min_duration as a reloption. > There is nothing really changed, the value of > log_autovacuum_min_duration being picked up at startup with > table_recheck_autovac() is used until the end for one table.
Thanks. You added this in the worker loop processing each table: /* * Check for config changes before processing each collected table. */ if (got_SIGHUP) { got_SIGHUP = false; ProcessConfigFile(PGC_SIGHUP); /* shutdown requested in config file? */ if (!AutoVacuumingActive()) break; } I think bailing out if autovac is disabled is a good idea; for instance, if this is a for-wraparound vacuum, we should keep running. My first reaction is that this part of the test ought to be moved down a screenful or so, until we've ran the recheck step and we have the for-wraparound flag in hand; only bail out if that one is not set. But on the other hand, maybe the worker will process some tables that are not for-wraparound in between some other tables that are, so that might turn out to be a bad idea as well. (Though I'm not 100% that it works that way; consider commit f51ead09df for instance.) Maybe the test to use for this is something along the lines of "if autovacuum was enabled before and is no longer enabled, bail out, otherwise keep running". This implies that we need to keep track of whether autovac was enabled at the start of the worker run. Another thing, but not directly related to this patch: while looking at the doc changes, I noticed that we don't have index entries for the reloptions we have; for instance, the word "fillfactor" doesn't appear in the bookindex.html page at all. Having these things all crammed in the CREATE TABLE page seems somewhat poor. I think we could improve on this by having a listing of settable parameters in a separate section, and have CREATE TABLE, ALTER TABLE, and the Server Configuration chapter link to that; we could also have index entries for these items. Of course, the simpler fix is to just add a few <indexterm> tags. As a note, there is no point to "Assert(foo != NULL)" tests when foo is later dereferenced in the same routine: the crash is going to happen anyway at the dereference, so it's just a LOC uselessly wasted. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers