Antonin Houska <a...@cybertec.at> wrote:

> It seems to be my bug. I'll check tomorrow.

I could reproduce the problem by adding sufficient sleep time to the
loop.

> Magnus Hagander <mag...@hagander.net> wrote:
>> I wonder if the else if (sleep > 0) at the bottom of throttle() should just
>> be a simple else and always run, resetting last_throttle?

I agree. In fact, I could simplify the code even more.

Since (elapsed + sleep) almost equals to GetCurrentIntegerTimestamp(), we can
use the following statement unconditionally (I think I tried too hard to avoid
calling GetCurrentIntegerTimestamp too often in the original patch):

throttled_last = GetCurrentIntegerTimestamp();

Thus we can also get rid of the "else" branch that clears both "sleep" and
"wait_result".

(The patch contains minor comment refinement that I found useful when seeing
the code after years.)

-- 
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de, http://www.cybertec.at

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
new file mode 100644
index ffc7e58..40b3c11
*** a/src/backend/replication/basebackup.c
--- b/src/backend/replication/basebackup.c
*************** throttle(size_t increment)
*** 1370,1395 ****
  		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.
  	 */
  	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();
  }
--- 1370,1385 ----
  		if (wait_result & WL_LATCH_SET)
  			CHECK_FOR_INTERRUPTS();
  	}
  
  	/*
! 	 * 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;
  
! 	/*
! 	 * Time interval for the remaining amount and possible next increments
! 	 * starts now.
! 	 */
! 	throttled_last = GetCurrentIntegerTimestamp();
  }
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to