Re: Error on pgbench logs
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Yugo NAGATA <nagata@sraoss.co.jp>
Cc: Fabien COELHO <coelho@cri.ensmp.fr>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, rulyox@gmail.com, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2021-06-15T05:17:14Z
Lists: pgsql-hackers
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
Commits
-
Fix pgbench timestamp bugs.
- 5614a0f78eaa 14.0 landed
- 0e39a608ed55 15.0 landed
-
pgbench: Improve time logic.
- 547f04e7348b 14.0 cited
-
Make [U]INT64CONST safe for use in #if conditions.
- 9d6b160d7db7 11.0 cited
-
Teach libpq to detect integer overflow in the row count of a PGresult.
- 2e70d6b5e99b 11.0 cited