Re: Fix around conn_duration in pgbench

On 2021/06/30 14:43, Yugo NAGATA wrote:
> On Wed, 30 Jun 2021 14:35:37 +0900
> Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp> wrote:
>
>> Hello Asif,
>>
>> On Tue, 29 Jun 2021 13:21:54 +0000
>> Asif Rehman <asifr(dot)rehman(at)gmail(dot)com> wrote:
>>
>>> The following review has been posted through the commitfest application:
>>> make installcheck-world: tested, passed
>>> Implements feature: tested, passed
>>> Spec compliant: tested, passed
>>> Documentation: not tested
>>>
>>> This patch looks fine to me. master 48cb244fb9
>>>
>>> The new status of this patch is: Ready for Committer
>>
>> Thank you for reviewing this patch!
>>
>> The previous patch included fixes about connection failures, but this part
>> was moved to another patch discussed in [1].
>>
>> [1] https://www.postgresql.org/message-id/alpine.DEB.2.22.394.2106181535400.3146194%40pseudo
>>
>> I attached the updated patach.
>
> I am sorry but I attached the other patch. Attached in this post
> is the latest patch.

case CSTATE_FINISHED:
+ /* per-thread last disconnection time is not measured */

Could you tell me why we don't need to do this measurement?

- /* no connection delay to record */
- thread->conn_duration = 0;
+ /* connection delay is measured globally between the barriers */

This comment is really correct? I was thinking that the measurement is not necessary here because this is the case where -C option is not specified.

done:
start = pg_time_now();
disconnect_all(state, nstate);
thread->conn_duration += pg_time_now() - start;

We should measure the disconnection time here only when -C option specified (i.e., is_connect variable is true)? Though, I'm not sure how much this change is helpful to reduce the performance overhead....

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION