Thread: PATCH: pgbench - logging aggregated info and transactions at the same time

Hi,

another thing that I find annoying on pgbench is that you can either log
aggregated summary (per interval) or detailed transaction info (possibly
sampled), but not both at the same time.

That's annoying because what I generally use the aggregated info, but
sometimes the transaction info would be handy too for verification and
further analysis.

Attached patch makes that possible by decoupling these two kinds of
logging. Up to now, '--agg-interval' option required '-l'. When both
options were used, aggregated data were logged. When only '-l' was used,
per-transaction info was logged. The patch makes those two options
independent.

When '-l' is used, per-transaction info (possibly for only a sample of
transactions, when --sample-rate is used) is written into
pgbench_log.PID.THREAD files.

When '--agg-interval' is used, aggregated info is collected and written
into pgbench_agg_log.PID.THREAD files.

It's possible to use all three options at the same time - in that case,
the sampling is only applied to the per-transaction logs. The aggregated
log will contain data from all the transactions.

This produces one log per thread, but combining this with the other
pgbench patch (log merge) should be trivial.


--
Tomas Vondra                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

> another thing that I find annoying on pgbench is that you can either log
> aggregated summary (per interval) or detailed transaction info (possibly
> sampled), but not both at the same time.

Here is a review.

Patch applied, compiled and run. Note that it needs a rebase after pgbench 
move to src/bin.


Overall comments:

This feature is a good think, i.e. it removes a current limitation which 
is not really logical.

I think that the user interface must be improved. I would suggest that 
option "-a/--aggregate-log" should trigger the aggregate log with a 
default aggregate interval of for instance 1 or 10 second(s), and 
"--aggregate-interval" would change the default interval.

Probably this patch could wait for the resolution of the pgbench merged 
log patch submitted in parallel as it seems to me that it interferes 
significantly with it.


Detailed comments:

In doLog, the sample rate test should be mixed in the tlogfile condition
so as to avoid calling rand if there is no logging anyway.
  if (tlogfile && sample_rate != 0 && ...)

The aggregate log file opening error refers to the "transaction" log file.

The documentation update is missing.

-- 
Fabien.

> In doLog, the sample rate test should be mixed in the tlogfile condition
> so as to avoid calling rand if there is no logging anyway.
>
>  if (tlogfile && sample_rate != 0 && ...)

Oops, wrong logic. Rather:
  if (tlogfile && (sample_rate == 0 || ... random stuff))

-- 
Fabien.