Bonjour Michaël,
+ /* flush remaining stats */
+ if (!logged && latency == 0.0)
+ logAgg(logfile, agg);
You are right, this is missing the final stats. Why the choice of
latency here for the check?
For me zero latency really says that there is no actual transaction to
count, so it is a good trigger for the closing call. I did not wish to add
a new "flush" parameter, or a specific function. I agree that it looks
strange, though.
That's just to make the difference between the case where doLog() is
called while processing the benchmark for the end of the transaction and
the case where doLog() is called once a thread ends, no?
Yes.
Wouldn't it be better to do a final push of the states once a thread
reaches CSTATE_FINISHED or CSTATE_ABORTED instead?
The call was already in place at the end of threadRun and had just become
ineffective. I did not wish to revisit its place and change the overall
structure, it is just a bug fix. I agree that it could be done differently
with the added logAgg function which could be called directly. Attached
another version which does that.
--
Fabien.
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index dc84b7b9b7..62f7994363 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3765,6 +3765,30 @@ executeMetaCommand(CState *st, pg_time_usec_t *now)
return CSTATE_END_COMMAND;
}
+/* print aggregated report to logfile */
+static void
+logAgg(FILE *logfile, StatsData *agg)
+{
+ fprintf(logfile, INT64_FORMAT " " INT64_FORMAT " %.0f %.0f %.0f %.0f",
+ agg->start_time,
+ agg->cnt,
+ agg->latency.sum,
+ agg->latency.sum2,
+ agg->latency.min,
+ agg->latency.max);
+ if (throttle_delay)
+ {
+ fprintf(logfile, " %.0f %.0f %.0f %.0f",
+ agg->lag.sum,
+ agg->lag.sum2,
+ agg->lag.min,
+ agg->lag.max);
+ if (latency_limit)
+ fprintf(logfile, " " INT64_FORMAT, agg->skipped);
+ }
+ fputc('\n', logfile);
+}
+
/*
* Print log entry after completing one transaction.
*
@@ -3793,36 +3817,19 @@ doLog(TState *thread, CState *st,
/* should we aggregate the results or not? */
if (agg_interval > 0)
{
+ pg_time_usec_t next;
+
/*
* Loop until we reach the interval of the current moment, and print
* any empty intervals in between (this may happen with very low tps,
* e.g. --rate=0.1).
*/
-
- while (agg->start_time + agg_interval <= now)
+ while ((next = agg->start_time + agg_interval * INT64CONST(1000000)) <= now)
{
- /* print aggregated report to logfile */
- fprintf(logfile, INT64_FORMAT " " INT64_FORMAT " %.0f %.0f %.0f %.0f",
- agg->start_time,
- agg->cnt,
- agg->latency.sum,
- agg->latency.sum2,
- agg->latency.min,
- agg->latency.max);
- if (throttle_delay)
- {
- fprintf(logfile, " %.0f %.0f %.0f %.0f",
- agg->lag.sum,
- agg->lag.sum2,
- agg->lag.min,
- agg->lag.max);
- if (latency_limit)
- fprintf(logfile, " " INT64_FORMAT, agg->skipped);
- }
- fputc('\n', logfile);
+ logAgg(logfile, agg);
/* reset data and move to next interval */
- initStats(agg, agg->start_time + agg_interval);
+ initStats(agg, next);
}
/* accumulate the current transaction */
@@ -6795,7 +6802,9 @@ done:
{
/* log aggregated but not yet reported transactions */
doLog(thread, state, &aggs, false, 0, 0);
+ logAgg(thread->logfile, &aggs);
}
+
fclose(thread->logfile);
thread->logfile = NULL;
}
