Re: Error on pgbench logs

Michaël-san, Yugo-san,

>> I am fine with this version, but I think it would be better if we have
>> a comment explaining what "tx" is for.

Yes. Done.

>> 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?

Ok. Done.

> Agreed on both points. From what I get, this code could be clarified
> much more,

I agree that the code is a little bit awkward.

> 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.

Yep.

> I am not completely sure that the result would be worth it either.

I'm not either.

> Let's document things and let's the readers know better the
> assumptions this area of the code relies on, for clarity.

Sure.

> 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.

Actually it would work if both are mixed: the code would aggregate a
sample. However it does not look very useful to do that, so it is
arbitrary forbidden. Not sure whether this is that useful to prevent this
use case.

> 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.

Yep.

Attached v4 improves comments and moves tx as an assert.

--
Fabien.