ITAGAKI Takahiro wrote: > > Alvaro Herrera <[EMAIL PROTECTED]> wrote: > > > > No, I meant a "while (sleep 1(or 10) and counter < longtime) check for > > > exit" instead of "sleep longtime". > > > > Ah; yes, what I was proposing (or thought about proposing, not sure if I > > posted it or not) was putting a upper limit of 10 seconds in the sleep > > (bgwriter sleeps 10 seconds if configured to not do anything). Though > > 10 seconds may seem like an eternity for systems like the ones Peter was > > talking about, where there is a script trying to restart the server as > > soon as the postmaster dies. > > Here is a patch for split-sleep of autovacuum_naptime. > > There are some other issues in CVS HEAD; We use the calculation > {autovacuum_naptime * 1000000} in launcher_determine_sleep(). > The result will be corrupted if we set autovacuum_naptime to >2147.
Ugh. How about this patch; this avoids the overflow issue altogether. I am not sure that this works on Win32 but it seems we are already using struct timeval elsewhere, so I don't see why it wouldn't work. > In another place, we use {autovacuum_naptime * 1000}, so we should > set the upper bound to INT_MAX/1000 instead of INT_MAX. > Incidentally, we've already had the same protections for > log_min_duration_statement and log_autovacuum. Hmm, yes, the naptime should have an upper bound of INT_MAX/1000. It doesn't seem worth the trouble of changing those places, when we know that such a high value of naptime is uselessly high. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Index: src/backend/postmaster/autovacuum.c =================================================================== RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/postmaster/autovacuum.c,v retrieving revision 1.49 diff -c -p -r1.49 autovacuum.c *** src/backend/postmaster/autovacuum.c 8 Jun 2007 21:21:28 -0000 1.49 --- src/backend/postmaster/autovacuum.c 13 Jun 2007 17:27:39 -0000 *************** *** 18,23 **** --- 18,24 ---- #include <signal.h> #include <sys/types.h> + #include <sys/time.h> #include <time.h> #include <unistd.h> *************** int autovacuum_vac_cost_limit; *** 73,78 **** --- 74,83 ---- int Log_autovacuum = -1; + + /* maximum sleep duration in the launcher */ + #define AV_SLEEP_QUANTUM 10 + /* Flags to tell if we are in an autovacuum process */ static bool am_autovacuum_launcher = false; static bool am_autovacuum_worker = false; *************** NON_EXEC_STATIC void AutoVacWorkerMain(i *** 197,203 **** NON_EXEC_STATIC void AutoVacLauncherMain(int argc, char *argv[]); static Oid do_start_worker(void); ! static uint64 launcher_determine_sleep(bool canlaunch, bool recursing); static void launch_worker(TimestampTz now); static List *get_database_list(void); static void rebuild_database_list(Oid newdb); --- 202,209 ---- NON_EXEC_STATIC void AutoVacLauncherMain(int argc, char *argv[]); static Oid do_start_worker(void); ! static void launcher_determine_sleep(bool canlaunch, bool recursing, ! struct timeval *nap); static void launch_worker(TimestampTz now); static List *get_database_list(void); static void rebuild_database_list(Oid newdb); *************** AutoVacLauncherMain(int argc, char *argv *** 487,493 **** for (;;) { ! uint64 micros; bool can_launch; TimestampTz current_time = 0; --- 493,499 ---- for (;;) { ! struct timeval nap; bool can_launch; TimestampTz current_time = 0; *************** AutoVacLauncherMain(int argc, char *argv *** 498,508 **** if (!PostmasterIsAlive(true)) exit(1); ! micros = launcher_determine_sleep(AutoVacuumShmem->av_freeWorkers != ! INVALID_OFFSET, false); ! /* Sleep for a while according to schedule */ ! pg_usleep(micros); /* the normal shutdown case */ if (avlauncher_shutdown_request) --- 504,542 ---- if (!PostmasterIsAlive(true)) exit(1); ! launcher_determine_sleep(AutoVacuumShmem->av_freeWorkers != ! INVALID_OFFSET, false, &nap); ! ! /* ! * Sleep for a while according to schedule. We only sleep in ! * AV_SLEEP_QUANTUM sec intervals, in order to promptly notice ! * postmaster death. ! */ ! while (nap.tv_sec > 0 || nap.tv_usec > 0) ! { ! uint32 sleeptime; ! ! sleeptime = nap.tv_usec; ! nap.tv_usec = 0; ! if (nap.tv_sec > 0) ! { ! sleeptime += Min(nap.tv_sec, AV_SLEEP_QUANTUM) * 1000000; ! nap.tv_sec -= Min(nap.tv_sec, AV_SLEEP_QUANTUM); ! } ! ! pg_usleep(sleeptime); ! ! /* ! * Emergency bailout if postmaster has died. This is to avoid the ! * necessity for manual cleanup of all postmaster children. ! */ ! if (!PostmasterIsAlive(true)) ! exit(1); ! ! if (avlauncher_shutdown_request || got_SIGHUP || got_SIGUSR1) ! break; ! } /* the normal shutdown case */ if (avlauncher_shutdown_request) *************** AutoVacLauncherMain(int argc, char *argv *** 647,662 **** } /* ! * Determine the time to sleep, in microseconds, based on the database list. * * The "canlaunch" parameter indicates whether we can start a worker right now, ! * for example due to the workers being all busy. */ ! static uint64 ! launcher_determine_sleep(bool canlaunch, bool recursing) { - long secs; - int usecs; Dlelem *elem; /* --- 681,695 ---- } /* ! * Determine the time to sleep, based on the database list. * * The "canlaunch" parameter indicates whether we can start a worker right now, ! * for example due to the workers being all busy. If this is false, we will ! * cause a long sleep, which will be interrupted when a worker exits. */ ! static void ! launcher_determine_sleep(bool canlaunch, bool recursing, struct timeval *nap) { Dlelem *elem; /* *************** launcher_determine_sleep(bool canlaunch, *** 667,689 **** */ if (!canlaunch) { ! secs = autovacuum_naptime; ! usecs = 0; } else if ((elem = DLGetTail(DatabaseList)) != NULL) { avl_dbase *avdb = DLE_VAL(elem); TimestampTz current_time = GetCurrentTimestamp(); TimestampTz next_wakeup; next_wakeup = avdb->adl_next_worker; TimestampDifference(current_time, next_wakeup, &secs, &usecs); } else { /* list is empty, sleep for whole autovacuum_naptime seconds */ ! secs = autovacuum_naptime; ! usecs = 0; } /* --- 700,727 ---- */ if (!canlaunch) { ! nap->tv_sec = autovacuum_naptime; ! nap->tv_usec = 0; } else if ((elem = DLGetTail(DatabaseList)) != NULL) { avl_dbase *avdb = DLE_VAL(elem); TimestampTz current_time = GetCurrentTimestamp(); TimestampTz next_wakeup; + long secs; + int usecs; next_wakeup = avdb->adl_next_worker; TimestampDifference(current_time, next_wakeup, &secs, &usecs); + + nap->tv_sec = secs; + nap->tv_usec = usecs; } else { /* list is empty, sleep for whole autovacuum_naptime seconds */ ! nap->tv_sec = autovacuum_naptime; ! nap->tv_usec = 0; } /* *************** launcher_determine_sleep(bool canlaunch, *** 696,715 **** * We only recurse once. rebuild_database_list should always return times * in the future, but it seems best not to trust too much on that. */ ! if (secs == 0L && usecs == 0 && !recursing) { rebuild_database_list(InvalidOid); ! return launcher_determine_sleep(canlaunch, true); } /* 100ms is the smallest time we'll allow the launcher to sleep */ ! if (secs <= 0L && usecs <= 100000) { ! secs = 0L; ! usecs = 100000; /* 100 ms */ } - - return secs * 1000000 + usecs; } /* --- 734,752 ---- * We only recurse once. rebuild_database_list should always return times * in the future, but it seems best not to trust too much on that. */ ! if (nap->tv_sec == 0L && nap->tv_usec == 0 && !recursing) { rebuild_database_list(InvalidOid); ! launcher_determine_sleep(canlaunch, true, nap); ! return; } /* 100ms is the smallest time we'll allow the launcher to sleep */ ! if (nap->tv_sec <= 0L && nap->tv_usec <= 100000) { ! nap->tv_sec = 0L; ! nap->tv_usec = 100000; /* 100 ms */ } } /*
---------------------------(end of broadcast)--------------------------- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate