I wrote:
> I was thinking that the launcher should only request fresh stats at wakeup,
> the workers could then reuse that file. This could be implemented by calling
> pgstat_clear_snapshot only at launcher wakeup and setting max stats age to
> to autovacuum_naptime for the workers.
>
Attached is a patch that increases the autovacuum stats age tolerance to
autovacuum_naptime. This is handled by autovac_refresh_stats() by not clearing
the stats snapshot unless nap time elapsed or explicitly forced by an error
or SIGHUP.
For the time being, I left the table vacuum recheck in place. Removing the
table_recheck_autovac function requires some further work. I have started on
this, but decided to defer until it is clear whether the whole approach is
acceptable or not.
regards,
Martin
*** a/src/backend/postmaster/autovacuum.c
--- b/src/backend/postmaster/autovacuum.c
***************
*** 44,54 ****
* Note that there can be more than one worker in a database concurrently.
* They will store the table they are currently vacuuming in shared memory, so
* that other workers avoid being blocked waiting for the vacuum lock for that
! * table. They will also reload the pgstats data just before vacuuming each
! * table, to avoid vacuuming a table that was just finished being vacuumed by
! * another worker and thus is no longer noted in shared memory. However,
! * there is a window (caused by pgstat delay) on which a worker may choose a
! * table that was already vacuumed; this is a bug in the current design.
*
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
--- 44,53 ----
* Note that there can be more than one worker in a database concurrently.
* They will store the table they are currently vacuuming in shared memory, so
* that other workers avoid being blocked waiting for the vacuum lock for that
! * table. There is a possibility that a worker might pick up a table that was
! * already vacuumed by another process. This isn't really a problem, as the
! * odds of this happening are low and the revacuum is made cheap by the use of
! * visibility map.
*
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
***************
*** 120,128 **** int autovacuum_vac_cost_limit;
int Log_autovacuum_min_duration = -1;
- /* how long to keep pgstat data in the launcher, in milliseconds */
- #define STATS_READ_DELAY 1000
-
/* Flags to tell if we are in an autovacuum process */
static bool am_autovacuum_launcher = false;
--- 119,124 ----
***************
*** 298,304 **** static void avl_sighup_handler(SIGNAL_ARGS);
static void avl_sigusr1_handler(SIGNAL_ARGS);
static void avl_sigterm_handler(SIGNAL_ARGS);
static void avl_quickdie(SIGNAL_ARGS);
! static void autovac_refresh_stats(void);
--- 294,300 ----
static void avl_sigusr1_handler(SIGNAL_ARGS);
static void avl_sigterm_handler(SIGNAL_ARGS);
static void avl_quickdie(SIGNAL_ARGS);
! static void autovac_refresh_stats(bool force);
***************
*** 500,509 **** AutoVacLauncherMain(int argc, char *argv[])
DatabaseList = NULL;
/*
! * Make sure pgstat also considers our stat data as gone. Note: we
! * mustn't use autovac_refresh_stats here.
*/
! pgstat_clear_snapshot();
/* Now we can allow interrupts again */
RESUME_INTERRUPTS();
--- 496,504 ----
DatabaseList = NULL;
/*
! * Make sure pgstat also considers our stat data as gone.
*/
! autovac_refresh_stats(true);
/* Now we can allow interrupts again */
RESUME_INTERRUPTS();
***************
*** 598,603 **** AutoVacLauncherMain(int argc, char *argv[])
--- 593,601 ----
if (got_SIGTERM)
break;
+ /* Refresh stats. Force it, if reloaded via SIGHUP */
+ autovac_refresh_stats(got_SIGHUP);
+
if (got_SIGHUP)
{
got_SIGHUP = false;
***************
*** 851,859 **** rebuild_database_list(Oid newdb)
int nelems;
HTAB *dbhash;
- /* use fresh stats */
- autovac_refresh_stats();
-
newcxt = AllocSetContextCreate(AutovacMemCxt,
"AV dblist",
ALLOCSET_DEFAULT_MINSIZE,
--- 849,854 ----
***************
*** 1078,1086 **** do_start_worker(void)
ALLOCSET_DEFAULT_MAXSIZE);
oldcxt = MemoryContextSwitchTo(tmpcxt);
- /* use fresh stats */
- autovac_refresh_stats();
-
/* Get a list of databases */
dblist = get_database_list();
--- 1073,1078 ----
***************
*** 2145,2158 **** do_autovacuum(void)
}
/*
! * Check whether pgstat data still says we need to vacuum this table.
! * It could have changed if something else processed the table while
! * we weren't looking.
! *
! * Note: we have a special case in pgstat code to ensure that the stats
! * we read are as up-to-date as possible, to avoid the problem that
! * somebody just finished vacuuming this table. The window to the race
! * condition is not closed but it is very small.
*/
MemoryContextSwitchTo(AutovacMemCxt);
tab = table_recheck_autovac(relid, table_toast_map);
--- 2137,2145 ----
}
/*
! * Check whether we still need to vacuum this table. It could have
! * changed if something else processed the table while we weren't
! * looking.
*/
MemoryContextSwitchTo(AutovacMemCxt);
tab = table_recheck_autovac(relid, table_toast_map);
***************
*** 2377,2385 **** table_recheck_autovac(Oid relid, HTAB *table_toast_map)
PgStat_StatDBEntry *dbentry;
bool wraparound;
- /* use fresh stats */
- autovac_refresh_stats();
-
shared = pgstat_fetch_stat_dbentry(InvalidOid);
dbentry = pgstat_fetch_stat_dbentry(MyDatabaseId);
--- 2364,2369 ----
***************
*** 2816,2844 **** AutoVacuumShmemInit(void)
* Refresh pgstats data for an autovacuum process
*
* Cause the next pgstats read operation to obtain fresh data, but throttle
! * such refreshing in the autovacuum launcher. This is mostly to avoid
! * rereading the pgstats files too many times in quick succession when there
! * are many databases.
! *
! * Note: we avoid throttling in the autovac worker, as it would be
! * counterproductive in the recheck logic.
*/
static void
! autovac_refresh_stats(void)
{
! if (IsAutoVacuumLauncherProcess())
! {
! static TimestampTz last_read = 0;
! TimestampTz current_time;
! current_time = GetCurrentTimestamp();
! if (!TimestampDifferenceExceeds(last_read, current_time,
! STATS_READ_DELAY))
! return;
! last_read = current_time;
! }
pgstat_clear_snapshot();
}
--- 2800,2822 ----
* Refresh pgstats data for an autovacuum process
*
* Cause the next pgstats read operation to obtain fresh data, but throttle
! * the refreshing so it only occurs at autovacuum_naptime intervals. This is
! * mostly to avoid rereading the pgstats files too many times in quick
! * succession when there are many databases.
*/
static void
! autovac_refresh_stats(bool force)
{
! static TimestampTz last_read = 0;
! TimestampTz current_time;
! current_time = GetCurrentTimestamp();
! if (!force && !TimestampDifferenceExceeds(last_read, current_time,
! autovacuum_naptime * 1000))
! return;
! last_read = current_time;
pgstat_clear_snapshot();
}
*** a/src/backend/postmaster/pgstat.c
--- b/src/backend/postmaster/pgstat.c
***************
*** 3388,3396 **** backend_read_statsfile(void)
* We set the minimum acceptable timestamp to PGSTAT_STAT_INTERVAL msec
* before now. This indirectly ensures that the collector needn't write
* the file more often than PGSTAT_STAT_INTERVAL. In an autovacuum
! * worker, however, we want a lower delay to avoid using stale data, so we
! * use PGSTAT_RETRY_DELAY (since the number of worker is low, this
! * shouldn't be a problem).
*
* Note that we don't recompute min_ts after sleeping; so we might end up
* accepting a file a bit older than PGSTAT_STAT_INTERVAL. In practice
--- 3388,3395 ----
* We set the minimum acceptable timestamp to PGSTAT_STAT_INTERVAL msec
* before now. This indirectly ensures that the collector needn't write
* the file more often than PGSTAT_STAT_INTERVAL. In an autovacuum
! * worker, however, we can reuse the stats file that was requested by the
! * launcher.
*
* Note that we don't recompute min_ts after sleeping; so we might end up
* accepting a file a bit older than PGSTAT_STAT_INTERVAL. In practice
***************
*** 3400,3406 **** backend_read_statsfile(void)
*/
if (IsAutoVacuumWorkerProcess())
min_ts = TimestampTzPlusMilliseconds(GetCurrentTimestamp(),
! -PGSTAT_RETRY_DELAY);
else
min_ts = TimestampTzPlusMilliseconds(GetCurrentTimestamp(),
-PGSTAT_STAT_INTERVAL);
--- 3399,3405 ----
*/
if (IsAutoVacuumWorkerProcess())
min_ts = TimestampTzPlusMilliseconds(GetCurrentTimestamp(),
! -autovacuum_naptime * 1000);
else
min_ts = TimestampTzPlusMilliseconds(GetCurrentTimestamp(),
-PGSTAT_STAT_INTERVAL);
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
