pgbench: Stop counting skipped transactions as soon as timer is exceeded.

MasaoFujii · MasaoFujii · commit b27d0cd3147e · 2021-09-10T01:28:51.000+09:00
When throttling is used, transactions that lag behind schedule by more than the latency limit are counted and reported as skipped. Previously, there was the case where pgbench counted all skipped transactions even if the timer specified in -T option was exceeded. This could take a very long time to do that especially when unrealistically high rate setting in -R option caused quite a lot of transactions that lagged behind schedule. This could prevent pgbench from ending immediately, and so pgbench could look like it got stuck to users. To fix the issue, this commit changes pgbench so that it stops counting skipped transactions as soon as the timer is exceeded. The timer can make pgbench end soon even when there are lots of skipped transactions that have not been counted yet. 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. Also this is not the issue that this commit newly introdues. There used to be the case where pgbench ended without counting all skipped transactions since before. Back-patch to v14. Per discussion, we decided not to bother back-patch to the stable branches because it's hard to imagine the issue happens in practice (with realistic setting). Author: Yugo Nagata, Fabien COELHO Reviewed-by: Greg Sabino Mullane, Fujii Masao Discussion: https://postgr.es/m/20210613040151.265ff59d832f835bbcf8b3ba@sraoss.co.jp
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
@@ -3232,31 +3232,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;
 					}
 				}
