Re: PATCH: pgbench - logging aggregated info and transactions at the same time - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: PATCH: pgbench - logging aggregated info and transactions at the same time
Date
Msg-id alpine.DEB.2.10.1505021038430.30090@sto
Whole thread Raw
In response to PATCH: pgbench - logging aggregated info and transactions at the same time  (Tomas Vondra <tomas.vondra@2ndquadrant.com>)
Responses Re: PATCH: pgbench - logging aggregated info and transactions at the same time  (Fabien COELHO <coelho@cri.ensmp.fr>)
List pgsql-hackers
> 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.



pgsql-hackers by date:

Previous
From: Fabien COELHO
Date:
Subject: Re: PATCH: pgbench - merging transaction logs
Next
From: Fabien COELHO
Date:
Subject: Re: PATCH: pgbench - logging aggregated info and transactions at the same time