Re: Fix around conn_duration in pgbench - Mailing list pgsql-hackers

On Fri, 30 Jul 2021 15:26:51 +0900
Fujii Masao <masao.fujii@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 <nagata@sraoss.co.jp>