Hello Fujii-san,
On Mon, 30 Aug 2021 23:36:30 +0900
Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
>
>
> On 2021/08/26 12:13, Yugo NAGATA wrote:
> > Ok. That makes sense. The output reports "including connections
> > establishing"
> > and "excluding connections establishing" regardless with -C, so we should
> > measure delays in the same way.
>
> On second thought, it's more reasonable and less confusing not to
> measure the disconnection delays at all? Since whether the benchmark result
> should include the disconnection delays or not is not undocumented,
> probably we cannot say strongly the current behavior (i.e., the disconnection
> delays are not measured) is a bug. Also since the result has not included
> the disconnection delays so far, the proposed change might slightly change
> the benchmark numbers reported, which might confuse the users.
> ISTM that at least it's unwise to change long-stable branches for this...
> Thought?
Ok. I agree with you that it is better to not change the behavior of pg13 or
before at least. As for pg14 or later, I wonder that we can change it when pg14
is released because the output was already change in the commit 547f04e734,
although, I am not persisting to measure disconnection delay since the effect
to tps would be very slight. At least, if we decide to not measure disconnection
delays, I think we should fix as so, like the attached patch.
> > I updated the patch for pg13 to measure disconnection delay when -C is not
> > specified. I attached the updated patch for pg13 as well as one for pg14
> > which is same as attached before.
>
> Thanks! I pushed the part of the patch, which gets rid of unnecessary
> measure of connection delays from pgbench.
Thank you!
Regards, Yugo Nagata
--
Yugo NAGATA <nag...@sraoss.co.jp>
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index bca136bdd5..6d895968fe 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -6538,7 +6538,11 @@ main(int argc, char **argv)
bench_start = thread->bench_start;
}
- /* XXX should this be connection time? */
+ /*
+ * All connections should be already closed in threadRun(), so this
+ * disconnect_all() will be a no-op, but clean up the connecions just
+ * to be sure.
+ */
disconnect_all(state, nclients);
/*
@@ -6827,9 +6831,7 @@ threadRun(void *arg)
}
done:
- start = pg_time_now();
disconnect_all(state, nstate);
- thread->conn_duration += pg_time_now() - start;
if (thread->logfile)
{
