postgres.git / summary / log / commit / refs

commit    d760d942c73c9a161feefb7dc4a0004b9b4e7787
Author:   Fujii Masao <fujii@postgresql.org>
Date:     Wed Sep 01 08:05:13 2021 +0000

    pgbench: Fix bug in measurement of disconnection delays.
    
    When -C/--connect option is specified, pgbench establishes and closes
    the connection for each transaction. In this case pgbench needs to
    measure the times taken for all those connections and disconnections,
    to include the average connection time in the benchmark result.
    But previously pgbench could not measure those disconnection delays.
    To fix the bug, this commit makes pgbench measure the disconnection
    delays whenever the connection is closed at the end of transaction,
    if -C/--connect option is specified.
    
    Back-patch to v14. Per discussion, we concluded not to back-patch to v13
    or before because changing that behavior in stable branches would
    surprise users rather than providing benefits.
    
    Author: Yugo Nagata
    Reviewed-by: Fabien COELHO, Tatsuo Ishii, Asif Rehman, Fujii Masao
    Discussion: https://postgr.es/m/20210614151155.a393bc7d8fed183e38c9f52a@sraoss.co.jp


 src/bin/pgbench/pgbench.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index a3843c2de4c..d4cbaac3615 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3552,8 +3552,12 @@ advanceConnectionState(TState *thread, CState *st, StatsData *agg)
 
 				if (is_connect)
 				{
+					pg_time_usec_t start = now;
+
+					pg_time_now_lazy(&start);
 					finishCon(st);
-					now = 0;
+					now = pg_time_now();
+					thread->conn_duration += now - start;
 				}
 
 				if ((st->cnt >= nxacts && duration <= 0) || timer_exceeded)
@@ -3577,6 +3581,19 @@ advanceConnectionState(TState *thread, CState *st, StatsData *agg)
 				 */
 			case CSTATE_ABORTED:
 			case CSTATE_FINISHED:
+
+				/*
+				 * Don't measure the disconnection delays here even if in
+				 * CSTATE_FINISHED and -C/--connect option is specified.
+				 * Because in this case all the connections that this thread
+				 * established are closed at the end of transactions and the
+				 * disconnection delays should have already been measured at
+				 * that moment.
+				 *
+				 * In CSTATE_ABORTED state, the measurement is no longer
+				 * necessary because we cannot report complete results anyways
+				 * in this case.
+				 */
 				finishCon(st);
 				return;
 		}
@@ -6557,7 +6574,11 @@ main(int argc, char **argv)
 			bench_start = thread->bench_start;
 	}
 
-	/* XXX should this be connection time? */
+	/*
+	 * All connections should be already closed in threadRun(), so this
+	 * disconnect_all() will be a no-op, but clean up the connecions just to
+	 * be sure. We don't need to measure the disconnection delays here.
+	 */
 	disconnect_all(state, nclients);
 
 	/*
@@ -6846,9 +6867,7 @@ threadRun(void *arg)
 	}
 
 done:
-	start = pg_time_now();
 	disconnect_all(state, nstate);
-	thread->conn_duration += pg_time_now() - start;
 
 	if (thread->logfile)
 	{


[parent: b7ad093d50e1]