Re: [BUGS] BUG #1588: pg_autovacuum sleep parameter overflow - Mailing list pgsql-patches

From Bruce Momjian
Subject Re: [BUGS] BUG #1588: pg_autovacuum sleep parameter overflow
Date
Msg-id 200505111452.j4BEqYB26325@candle.pha.pa.us
Whole thread Raw
Responses Re: [BUGS] BUG #1588: pg_autovacuum sleep parameter overflow
Re: [BUGS] BUG #1588: pg_autovacuum sleep parameter overflow
Re: [BUGS] BUG #1588: pg_autovacuum sleep parameter overflow
List pgsql-patches
Lionel Bouton wrote:
>
> The following bug has been logged online:
>
> Bug reference:      1588
> Logged by:          Lionel Bouton
> Email address:      lionel-subscription@bouton.name
> PostgreSQL version: 8.0.1
> Operating system:   Linux 2.6, glibc 2.3.4
> Description:        pg_autovacuum sleep parameter overflow
> Details:
>
> pg_autovacuum doesn't sleep when the -s parameter is set above 2147
> (PostgreSQL then is under constant stress), seems like a 32-bit signed
> integer overflow (the time in µs then gets past 2^31-1) somewhere.
>
> This worked with 7.4.6 (at least with -s 3600).

Yes, this is a bug.  Right now, pg_autovacuum uses pg_usleep(), which
accepts a 'long' value representing the number of microseconds to sleep.
Because long is 32-bits on many platforms, it overflows around 2 billion
microseconds, which is 2147 seconds.  The comment in pg_usleep()
verifies it:

   * On machines where "long" is 32 bits, the maximum delay is ~2000 seconds.

Here is a proposed patch that will fix it.  There is no reason to use
pg_usleep in pg_autovacuum in this area, so we just use sleep().  We
don't need micro-second accuracy, and we don't need Win32 to handle
signals that arrive during the sleep.

However, I am now wondering if we should change pg_usleep() to take a
double rather than long.  This would avoid such problems in the future
in other places in our code.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: contrib/pg_autovacuum/pg_autovacuum.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/pg_autovacuum/pg_autovacuum.c,v
retrieving revision 1.31
diff -c -c -r1.31 pg_autovacuum.c
*** contrib/pg_autovacuum/pg_autovacuum.c    19 Apr 2005 03:35:15 -0000    1.31
--- contrib/pg_autovacuum/pg_autovacuum.c    11 May 2005 14:43:34 -0000
***************
*** 1749,1755 ****
              fflush(LOGOUTPUT);
          }

!         pg_usleep(sleep_secs * 1000000);    /* Larger Pause between outer loops */

          gettimeofday(&then, 0); /* Reset time counter */

--- 1749,1764 ----
              fflush(LOGOUTPUT);
          }

!         /* Larger Pause between outer loops */
!         /*
!          *    pg_usleep() is wrong here because its maximum is ~2000 seconds,
!          *    and we don't need signal interruptability on Win32 here.
!          */
! #ifndef WIN32
!         sleep(sleep_secs);            /* Unix sleep is seconds */
! #else
!         sleep(sleep_secs * 1000);    /* Win32 sleep() is milliseconds */
! #endif

          gettimeofday(&then, 0); /* Reset time counter */


pgsql-patches by date:

Previous
From: a_ogawa
Date:
Subject: AllocSetReset improvement
Next
From: Andrew Dunstan
Date:
Subject: plperl and pltcl installcheck targets