Re: Error on pgbench logs

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>
Cc: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, rulyox(at)gmail(dot)com, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Error on pgbench logs
Date: 2021-06-15 22:53:30
Message-ID: YMkvajCe4ULJM3SE@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jun 15, 2021 at 09:53:06PM +0900, Yugo NAGATA wrote:
> I am fine with this version, but I think it would be better if we have a comment
> explaining what "tx" is for.
>
> Also, how about adding Assert(tx) instead of using "else if (tx)" because
> we are assuming that tx is always true when agg_interval is not used, right?

Agreed on both points. From what I get, this code could be clarified
much more, and perhaps partially refactored to have less spaghetti
code between the point where we call it at the end of a thread or when
gathering stats of a transaction mid-run, but that's not something to
do post-beta1. I am not completely sure that the result would be
worth it either.

Let's document things and let's the readers know better the
assumptions this area of the code relies on, for clarity. The
dependency between agg_interval and sample_rate is one of those
things, somebody needs now to look at the option parsing why only one
is possible at the time. Using an extra tx flag to track what to do
after the loop for the aggregate print to the log file is an
improvement in this direction.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-06-15 23:03:41 Improving isolationtester's data output
Previous Message Daniel Gustafsson 2021-06-15 22:08:58 Re: Support for NSS as a libpq TLS backend