On Fri, 11 Jun 2021 16:09:10 +0200 (CEST)
Fabien COELHO <coelho@cri.ensmp.fr> wrote:
>
> Bonjour Michaël,
>
> >> + /* flush remaining stats */
> >> + if (!logged && latency == 0.0)
> >> + logAgg(logfile, agg);
> >
> > You are right, this is missing the final stats. Why the choice of
> > latency here for the check?
>
> For me zero latency really says that there is no actual transaction to
> count, so it is a good trigger for the closing call. I did not wish to add
> a new "flush" parameter, or a specific function. I agree that it looks
> strange, though.
It will not work if the transaction is skipped, in which case latency is 0.0.
It would work if we check also "skipped" as bellow.
+ if (!logged && !skipped && latency == 0.0)
However, it still might not work if the latency is so small so that we could
observe latency == 0.0. I observed this when I used a script that contained
only a meta command. This is not usual and would be a corner case, though.
> > Wouldn't it be better to do a final push of the states once a thread
> > reaches CSTATE_FINISHED or CSTATE_ABORTED instead?
>
> The call was already in place at the end of threadRun and had just become
> ineffective. I did not wish to revisit its place and change the overall
> structure, it is just a bug fix. I agree that it could be done differently
> with the added logAgg function which could be called directly. Attached
> another version which does that.
/* log aggregated but not yet reported transactions */
doLog(thread, state, &aggs, false, 0, 0);
+ logAgg(thread->logfile, &aggs);
I think we don't have to call doLog() before logAgg(). If we call doLog(),
we will count an extra transaction that is not actually processed because
accumStats() is called in this.
Regards,
Yugo Nagata
--
Yugo NAGATA <nagata@sraoss.co.jp>
