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@sraoss.co.jp> wrote:
> 
>> Hello Asif,
>>
>> On Tue, 29 Jun 2021 13:21:54 +0000
>> Asif Rehman <asifr.rehman@gmail.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