Hello Fabien,

On Sun, 13 Jun 2021 08:56:59 +0200 (CEST)
Fabien COELHO <coe...@cri.ensmp.fr> wrote:

> > I attached a patch for this fix.
> 
> The patch mostly works for me, and I agree that the bench should not be in 
> a loop on any parameters, even when "crazy" parameters are given…
> 
> However I'm not sure this is the right way to handle this issue.
> 
> The catch-up loop can be dropped and the automaton can loop over itself to 
> reschedule. Doing that as the attached fixes this issue and also makes 
> progress reporting work proprely in more cases, and reduces the number of 
> lines of code. I did not add a test case because time sensitive tests have 
> been removed (which is too bad, IMHO).

I agree with your way to fix. However, the progress reporting didn't work
because we cannot return from advanceConnectionState to threadRun and just
break the loop.

+                                               /* otherwise loop over 
PREPARE_THROTTLE */
                                                break;

I attached the fixed patch that uses return instead of break, and I confirmed
that this made the progress reporting work property.

Regards,
Yugo Nagata

-- 
Yugo NAGATA <nag...@sraoss.co.jp>
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index dc84b7b9b7..8f5d000938 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3223,32 +3223,31 @@ advanceConnectionState(TState *thread, CState *st, StatsData *agg)
 				/*
 				 * If --latency-limit is used, and this slot is already late
 				 * so that the transaction will miss the latency limit even if
-				 * it completed immediately, skip this time slot and schedule
-				 * to continue running on the next slot that isn't late yet.
-				 * But don't iterate beyond the -t limit, if one is given.
+				 * it completed immediately, skip this time slot and loop to
+				 * reschedule.
 				 */
 				if (latency_limit)
 				{
 					pg_time_now_lazy(&now);
 
-					while (thread->throttle_trigger < now - latency_limit &&
-						   (nxacts <= 0 || st->cnt < nxacts))
+					if (thread->throttle_trigger < now - latency_limit)
 					{
 						processXactStats(thread, st, &now, true, agg);
-						/* next rendez-vous */
-						thread->throttle_trigger +=
-							getPoissonRand(&thread->ts_throttle_rs, throttle_delay);
-						st->txn_scheduled = thread->throttle_trigger;
-					}
 
-					/*
-					 * stop client if -t was exceeded in the previous skip
-					 * loop
-					 */
-					if (nxacts > 0 && st->cnt >= nxacts)
-					{
-						st->state = CSTATE_FINISHED;
-						break;
+						/* stop client if -T/-t was exceeded. */
+						if (timer_exceeded || (nxacts > 0 && st->cnt >= nxacts))
+							/*
+							 * For very unrealistic rates under -T, some skipped
+							 * transactions are not counted because the catchup
+							 * loop is not fast enough just to do the scheduling
+							 * and counting at the expected speed.
+							 *
+							 * We do not bother with such a degenerate case.
+							 */
+							st->state = CSTATE_FINISHED;
+
+						/*otherwise loop over PREPARE_THROTTLE */
+						return;
 					}
 				}
 

Reply via email to