On 1/20/17 12:40 AM, Amit Khandekar wrote:
My impression was that postmaster is supposed to just do a minimal
work of starting auto-vacuum launcher if not already. And, the work of
ensuring all the things keep going is the job of auto-vacuum launcher.
There's already a ton of logic in the launcher... ISTM it'd be nice to
not start adding additional logic to the postmaster. If we had a generic
need for rate limiting launching of things maybe it wouldn't be that
bad, but AFAIK we don't.
That limits us to launching the
autovacuum launcher at most six times a minute when autovacuum = off.
You could argue that defeats the point of the SendPostmasterSignal in
SetTransactionIdLimit, but I don't think so. If vacuuming the oldest
database took less than 10 seconds, then we won't vacuum the
next-oldest database until we hit the next 64kB transaction ID
boundary, but that can only cause a problem if we've got so many
databases that we don't get to them all before we run out of
transaction IDs, which is almost unthinkable. If you had a ten
million tiny databases that all crossed the threshold at the same
instant, it would take you 640 million transaction IDs to visit them
all. If you also had autovacuum_freeze_max_age set very close to the
upper limit for that variable, you could conceivably have the system
shut down before all of those databases were reached. But that's a
pretty artificial scenario. If someone has that scenario, perhaps
they should consider more sensible configuration choices.
Yeah this logic makes sense ...
I'm not sure that's true in the case of a significant number of
databases and a very high XID rate, but I might be missing something. In
any case I agree it's not worth worrying about. If you've disabled
autovac you're already running with scissors.
But I guess , from looking at the code, it seems that it was carefully
made sure that in case of auto-vacuum off, we should clean up all
databases as fast as possible with multiple workers cleaning up
multiple tables in parallel.
Instead of autovacuum launcher and worker together making sure that
the cycle of iterations keep on running, I was thinking the
auto-vacuum launcher itself should make sure it does not spawn another
worker on the same database if it did nothing. But that seemed pretty
invasive.
IMHO we really need some more sophistication in scheduling for both
launcher and worker. Somewhere on my TODO is allowing the worker to call
a user defined SELECT to get a prioritized list, but since the launcher
doesn't connect to a database that wouldn't work. What we could do
rather simply is honor adl_next_worker in the logic that looks for
freeze, something like the attached.
On another note, does anyone else find the database selection logic
rather difficult to trace through? The logic is kinda spread throughout
several functions. The naming of rebuild_database_list() and
get_database_list() is rather confusing too.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)
diff --git a/src/backend/postmaster/autovacuum.c
b/src/backend/postmaster/autovacuum.c
index 264298e8a9..34969b058f 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -1066,6 +1066,43 @@ db_comparator(const void *a, const void *b)
}
/*
+ * Returns true if database has been processed recently (less than
+ * autovacuum_naptime seconds ago).
+ */
+static boolean
+recent_activity(Oid datid)
+{
+ dlist_iter iter;
+
+ dlist_reverse_foreach(iter, &DatabaseList)
+ {
+ avl_dbase *dbp = dlist_container(avl_dbase, adl_node,
iter.cur);
+
+ if (dbp->adl_datid == datid)
+ {
+ /*
+ * Skip this database if its next_worker value falls
between
+ * the current time and the current time plus naptime.
+ */
+ if (!TimestampDifferenceExceeds(dbp->adl_next_worker,
+
current_time, 0) &&
+ !TimestampDifferenceExceeds(current_time,
+
dbp->adl_next_worker,
+
autovacuum_naptime * 1000))
+ return true;
+ return false;
+
+ }
+ }
+
+ /*
+ * Didn't find the database on the internal list, so it must be new.
Skip
+ * it for now.
+ */
+ return true;
+}
+
+/*
* do_start_worker
*
* Bare-bones procedure for starting an autovacuum worker from the launcher.
@@ -1162,24 +1199,33 @@ do_start_worker(void)
foreach(cell, dblist)
{
avw_dbase *tmp = lfirst(cell);
- dlist_iter iter;
/* Check to see if this one is at risk of wraparound */
if (TransactionIdPrecedes(tmp->adw_frozenxid, xidForceLimit))
{
- if (avdb == NULL ||
+ /*
+ * We won't re-launch if this database was recently hit
by a
+ * worker. Otherwise, we want to find the database with
the oldest
+ * frozen XID. Previously, we only looked for the
oldest frozen
+ * XID, but by skipping recently hit databases we'll
make sure we
+ * hit everything that's in risk of wraparound.
+ */
+ if (!recent_activity(tmp->adw_datid) &&
+ (avdb == NULL ||
TransactionIdPrecedes(tmp->adw_frozenxid,
-
avdb->adw_frozenxid))
+
avdb->adw_frozenxid)))
avdb = tmp;
for_xid_wrap = true;
continue;
}
else if (for_xid_wrap)
continue; /* ignore not-at-risk
DBs */
+ /* Same as above, but for multixacts. */
else if (MultiXactIdPrecedes(tmp->adw_minmulti,
multiForceLimit))
{
- if (avdb == NULL ||
- MultiXactIdPrecedes(tmp->adw_minmulti,
avdb->adw_minmulti))
+ if (!recent_activity(tmp->adw_datid) &&
+ (avdb == NULL ||
+ MultiXactIdPrecedes(tmp->adw_minmulti,
avdb->adw_minmulti)))
avdb = tmp;
for_multi_wrap = true;
continue;
@@ -1204,28 +1250,7 @@ do_start_worker(void)
* selected, but that pgstat hasn't gotten around to updating
the last
* autovacuum time yet.
*/
- skipit = false;
-
- dlist_reverse_foreach(iter, &DatabaseList)
- {
- avl_dbase *dbp = dlist_container(avl_dbase, adl_node,
iter.cur);
-
- if (dbp->adl_datid == tmp->adw_datid)
- {
- /*
- * Skip this database if its next_worker value
falls between
- * the current time and the current time plus
naptime.
- */
- if
(!TimestampDifferenceExceeds(dbp->adl_next_worker,
-
current_time, 0) &&
-
!TimestampDifferenceExceeds(current_time,
-
dbp->adl_next_worker,
-
autovacuum_naptime * 1000))
- skipit = true;
-
- break;
- }
- }
+ skipit = recent_activity(tmp->adw_datid)
if (skipit)
continue;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
