Re: Sub-millisecond [autovacuum_]vacuum_cost_delay broken - Mailing list pgsql-hackers

Thomas Munro <thomas.munro@gmail.com> writes:
> On Fri, Mar 10, 2023 at 11:02 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The caf626b2c code would only work well on platforms that have
>> microsecond-based sleep primitives, so it was already not too portable.

> Also, the previous coding was already b0rked, because pg_usleep()
> rounds up to milliseconds on Windows (with a surprising formula for
> rounding), and also the whole concept seems to assume things about
> schedulers that aren't really universally true.  If we actually cared
> about high res times maybe we should be using nanosleep and tracking
> the drift?  And spreading it out a bit.  But I don't know.

Yeah, I was wondering about trying to make it a closed-loop control,
but I think that'd be huge overkill considering what the mechanism is
trying to accomplish.

A minimalistic fix could be as attached.  I'm not sure if it's worth
making the state variable global so that it can be reset to zero in
the places where we zero out VacuumCostBalance etc.  Also note that
this is ignoring the VacuumSharedCostBalance stuff, so you'd possibly
have the extra delay accumulating in unexpected places when there are
multiple workers.  But I really doubt it's worth worrying about that.

Is it reasonable to assume that all modern platforms can time
millisecond delays accurately?  Ten years ago I'd have suggested
truncating the delay to a multiple of 10msec and using this logic
to track the remainder, but maybe now that's unnecessary.

            regards, tom lane

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 2e12baf8eb..64d3c59709 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -2229,14 +2229,30 @@ vacuum_delay_point(void)
     /* Nap if appropriate */
     if (msec > 0)
     {
+        /*
+         * Since WaitLatch can only wait in units of milliseconds, carry any
+         * residual fractional msec in a static variable, and accumulate it to
+         * add into the wait interval next time.
+         */
+        static double residual_msec = 0;
+        long        lmsec;
+
+        msec += residual_msec;
+
         if (msec > VacuumCostDelay * 4)
             msec = VacuumCostDelay * 4;
 
-        (void) WaitLatch(MyLatch,
-                         WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
-                         msec,
-                         WAIT_EVENT_VACUUM_DELAY);
-        ResetLatch(MyLatch);
+        lmsec = floor(msec);
+        residual_msec = msec - lmsec;
+
+        if (lmsec > 0)
+        {
+            (void) WaitLatch(MyLatch,
+                             WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+                             lmsec,
+                             WAIT_EVENT_VACUUM_DELAY);
+            ResetLatch(MyLatch);
+        }
 
         VacuumCostBalance = 0;