On Fri, 30 Jul 2021 15:26:51 +0900 Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
> > > On 2021/07/30 14:43, Yugo NAGATA wrote: > > This patch fixes three issues of connection time measurement: > > > > 1. The initial connection time is measured and stored into conn_duration > > but the result is never used. > > 2. The disconnection time are not measured even when -C is specified. > > 3. The disconnection time measurement at the end of threadRun() is > > not necessary. > > > > The first one exists only in v14 and master, but others are also in v13 and > > before. So, I think we can back-patch these fixes. > > Yes, you're right. > > > > >> But the patch fails to be back-patched to v13 or before because > >> pgbench's time logic was changed by commit 547f04e734. > >> Do you have the patches that can be back-patched to v13 or before? > > > > I attached the patch for v13. > > Thanks for the patch! > > + /* > + * In CSTATE_FINISHED state, this finishCon() > is a no-op > + * under -C/--connect because all the > connections that this > + * thread established should have already been > closed at the end > + * of transactions. So we don't need to measure > the disconnection > + * delays here. > > In v13, the disconnection time needs to be measured in CSTATE_FINISHED > because the connection can be closed here when -C is not specified? When -C is not specified, the disconnection time is not measured even in the patch for v14+. I don't think it is a problem because the disconnection delay at the end of benchmark almost doesn't affect the tps. > > /* tps is about actually executed transactions */ > tps_include = ntx / time_include; > tps_exclude = ntx / > (time_include - (INSTR_TIME_GET_DOUBLE(conn_total_time) / > nclients)); > > BTW, this is a bit different topic from the patch, but in v13, > tps excluding connection establishing is calculated in the above way. > The total connection time is divided by the number of clients, > but why do we need this division? Do you have any idea? conn_total_time is a sum of connection delays measured over all threads that are running concurrently. So, we try to get the average connection delays by dividing by the number of clients, I think. However, I am not sure this is the right way though, and in fact it was revised in the recent commit so that we don't report the "tps excluding connection establishing" especially when -C is specified. Regards, Yugo Nagata -- Yugo NAGATA <nag...@sraoss.co.jp>
