On 2021/09/07 18:24, Fabien COELHO wrote:
Hello Fujii-san,
Stop counting skipped transactions under -T as soon as the timer is exceeded.
Because otherwise it can take a very long time to count all of them especially
when quite a lot of them happen with unrealistically high rate setting in -R,
which would prevent pgbench from ending immediately. Because of this behavior,
note that there is no guarantee that all skipped transactions are counted under
-T though there is under -t. This is OK in practice because it's very unlikely
to happen with realistic setting.
Ok, I find this text quite clear.
Thanks for the check! So attached is the updated version of the patch.
One question is; which version do we want to back-patch to?
If we consider it a "very minor bug fix" which is triggered by somehow unrealistic
options, so I'd say 14 & dev, or possibly only dev.
Agreed. Since it's hard to imagine the issue happens in practice,
we don't need to bother back-patch to the stable branches.
So I'm thinking to commit the patch to 15dev and 14.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 4c9952a85a..433abd954b 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3233,31 +3233,36 @@ 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;
+ /*
+ * Finish client if -T or -t
was exceeded.
+ *
+ * Stop counting skipped
transactions under -T as soon
+ * as the timer is exceeded.
Because otherwise it can
+ * take a very long time to
count all of them
+ * especially when quite a lot
of them happen with
+ * unrealistically high rate
setting in -R, which
+ * would prevent pgbench from
ending immediately.
+ * Because of this behavior,
note that there is no
+ * guarantee that all skipped
transactions are counted
+ * under -T though there is
under -t. This is OK in
+ * practice because it's very
unlikely to happen with
+ * realistic setting.
+ */
+ if (timer_exceeded || (nxacts >
0 && st->cnt >= nxacts))
+ st->state =
CSTATE_FINISHED;
+
+ /* Go back to top of loop with
CSTATE_PREPARE_THROTTLE */
break;
}
}
