Fix base backup rate limiting in presence of slow i/o

When source i/o on disk was too slow compared to the rate limiting
specified, the system could end up with a negative value for sleep that
it never got out of, which caused rate limiting to effectively be
turned off.

Discussion: https://postgr.es/m/CABUevEy_-e0YvL4ayoX8bH_Ja9w%2BBHoP6jUgdxZuG2nEj3uAfQ%40mail.gmail.com

Analysis by me, patch by Antonin Houska

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index ffc7e586dba712748dd13b31fdaac2cb91504ae1..40b3c117e8fc4ef5381edbc2fe52800cc3d23497 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -1370,26 +1370,16 @@ throttle(size_t increment)
                if (wait_result & WL_LATCH_SET)
                        CHECK_FOR_INTERRUPTS();
        }
-       else
-       {
-               /*
-                * The actual transfer rate is below the limit.  A negative value
-                * would distort the adjustment of throttled_last.
-                */
-               wait_result = 0;
-               sleep = 0;
-       }
 
        /*
-        * Only a whole multiple of throttling_sample was processed. The rest will
-        * be done during the next call of this function.
+        * As we work with integers, only whole multiple of throttling_sample was
+        * processed. The rest will be done during the next call of this function.
         */
        throttling_counter %= throttling_sample;
 
-       /* Once the (possible) sleep has ended, new period starts. */
-       if (wait_result & WL_TIMEOUT)
-               throttled_last += elapsed + sleep;
-       else if (sleep > 0)
-               /* Sleep was necessary but might have been interrupted. */
-               throttled_last = GetCurrentIntegerTimestamp();
+       /*
+        * Time interval for the remaining amount and possible next increments
+        * starts now.
+        */
+       throttled_last = GetCurrentIntegerTimestamp();
 }