On Sun, Jun 13, 2021 at 03:07:51AM +0900, Yugo NAGATA wrote:
> 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.
Hmm. I am not sure to completely follow the idea here. It would be
good to make this code less confusing than it is now.
> /* 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.
Yes, calling both is weird. Is using logAgg() directly in the context
actually right when it comes to sample_rate? We may not log anything
on HEAD if sample_rate is enabled, but we would finish by logging
something all the time with this patch. If I am following this code
correctly, we don't care about accumStats() in the code path of a
thread we are done with, right?
--
Michael