(Note: I've not read the patch, so this is not an opinion about its
correctness.)

As Fabien anayzed the problem was that pgbench simply uses wrong
variable: nthreads (number of threads, specified by "-j" option)
vs. nclients (number of clients, specified by "-c" option).

@@ -2616,7 +2616,7 @@ printResults(int ttype, int64 normal_xacts, int nclients,
        time_include = INSTR_TIME_GET_DOUBLE(total_time);
        tps_include = normal_xacts / time_include;
        tps_exclude = normal_xacts / (time_include -
-                                               
(INSTR_TIME_GET_DOUBLE(conn_total_time) / nthreads));
+                                               
(INSTR_TIME_GET_DOUBLE(conn_total_time) / nclients));

Here conn_total_time is the sum of time to establish connection to
PostgreSQL. Since establishing connections to PostgreSQL is done in
serial rather than in parallel, conn_total_time should have been
divided by nclients.

After some more thinking and looking again at the connection code, I revise slightly my diagnostic:

- the amount of parallelism is "nclients", as discussed above, when reconnecting on each transaction (-C) because the connections are managed in parallel from doCustom,

* BUT *

- if there is no reconnections (not -C) the connections are performed in threadRun in a sequential way, all clients wait for the connections of other clients in the same thread before starting processing transactions, so "nthreads" is the right amount of parallelism in this case.

So on second thought the formula should rather be:

  ...  / (is_connect? nclients: nthreads)

Here is a v2 with this formula. Note that it does not take into account thread creation time, which might be significant, the start and end of a pgbench run are quite fuzzy.

I've removed the doCustom parameter change as it is included in a larger patch I submitted about reworking stat collections in pgbench, so this attached patch is bug fix only.

For the record, I still plainly disagree with the idea of shipping a performance measuring tool which knowingly displays wrong and optimistic figures under some conditions.

--
Fabien.
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 6ae1b86..a6763f6 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -2616,7 +2616,8 @@ printResults(int ttype, int64 normal_xacts, int nclients,
 	time_include = INSTR_TIME_GET_DOUBLE(total_time);
 	tps_include = normal_xacts / time_include;
 	tps_exclude = normal_xacts / (time_include -
-						(INSTR_TIME_GET_DOUBLE(conn_total_time) / nthreads));
+						(INSTR_TIME_GET_DOUBLE(conn_total_time) /
+						 (is_connect? nclients: nthreads)));
 
 	if (ttype == 0)
 		s = "TPC-B (sort of)";
@@ -3462,7 +3463,7 @@ main(int argc, char **argv)
 
 		/* thread level stats */
 		throttle_lag += thread->throttle_lag;
-		throttle_latency_skipped += threads->throttle_latency_skipped;
+		throttle_latency_skipped += thread->throttle_latency_skipped;
 		latency_late += thread->latency_late;
 		if (throttle_lag_max > thread->throttle_lag_max)
 			throttle_lag_max = thread->throttle_lag_max;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to