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; } }