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

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org>
Subject: Re: PATCH: pgbench - logging aggregated info and transactions at the same time
Date: 2015-05-02 14:44:22
Message-ID: alpine.DEB.2.10.1505021038430.30090@sto
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2015-05-02 14:49:53 Re: PATCH: pgbench - logging aggregated info and transactions at the same time
Previous Message Fabien COELHO 2015-05-02 13:30:37 Re: PATCH: pgbench - merging transaction logs