Re: Why are wait events not reported even though it reads/writes a timeline history file?

From: Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Why are wait events not reported even though it reads/writes a timeline history file?
Date: 2020-04-28 02:10:00
Message-ID: 3480dda4f5a043d8819de9e8653dc331@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2020-04-27 12:25, Fujii Masao wrote:
> On 2020/04/24 11:29, Masahiro Ikeda wrote:
>> Hi,
>>
>> There are two unexpected codes for me about wait events for timeline
>> history file.
>> Please let me know your thoughts whether if we need to change.
>>
>>
>> 1. readTimeLineHistory() function in timeline.c
>>
>> The readTimeLineHistory() reads a timeline history file,
>> but it doesn't report “WAIT_EVENT_TIMELINE_HISTORY_READ".
>
> Yeah, this sounds strange.
>
>> In my understanding, sscanf() is blocking read.
>> So, it's important to report a wait event.
>
> Shouldn't the wait event be reported during fgets() rather than
> sscanf()?
>
>> 2. writeTimeLineHistory() function in timeline.c
>>
>> The writeTimeLineHistory() function may write a timeline history file
>> twice,
>> but it reports “WAIT_EVENT_TIMELINE_HISTORY_WRITE" only once.
>>
>> It makes sense to report a wait event twice, because both of them use
>> write().
>
> Yes.
>
>> I attached a patch to mention the code line number.
>>
>>
>> I checked the commit log which "WAIT_EVENT_TIMELINE_HISTORY_READ" and
>> "WAIT_EVENT_TIMELINE_HISTORY_WRITE" are committed and the discussion
>> about it.
>> But I can't find the reason.
>>
>> Please give me your comments.
>> If we need to change, I can make a patch to fix them.
>
> Thanks! I agree to fix those issues.

Thanks for the comments. I attach a patch to fix those issues.
Please review it.

>> By the way, which is correct "timeline's history file" or "timeline
>> history file"?
>> The timeline.c has both. In my understanding, the latter is correct.
>> If so, I will modify together.
>
> Maybe both are correct?? I have no strong opinion about this.

OK, I didn't fix it at this time.

Regards,
--
Masahiro Ikeda

Attachment Content-Type Size
0001-Fix-to-report-wait-events-about-timeline-history-fil.patch text/x-diff 1.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Fetter 2020-04-28 02:32:34 Re: Proposing WITH ITERATIVE
Previous Message Amit Kapila 2020-04-28 02:08:57 Re: WAL usage calculation patch