From: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Why are wait events not reported even though it reads/writes a timeline history file?
Date: Tue, 28 Apr 2020 11:10:00 +0900
Message-ID: <3480dda4f5a043d8819de9e8653dc331@oss.nttdata.com> (raw)
In-Reply-To: <c2bed73d-2044-8ab1-075a-75ce0316a814@oss.nttdata.com>
References: <d11b0c910b63684424e06772eb844ab5@oss.nttdata.com>
<c2bed73d-2044-8ab1-075a-75ce0316a814@oss.nttdata.com>
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
Attachments:
[text/x-diff] 0001-Fix-to-report-wait-events-about-timeline-history-fil.patch (1.6K, ../3480dda4f5a043d8819de9e8653dc331@oss.nttdata.com/2-0001-Fix-to-report-wait-events-about-timeline-history-fil.patch)
download | inline diff:
From 9e5165673714b929a76bf39792f04ea5e348f0a3 Mon Sep 17 00:00:00 2001
From: Masahiro Ikeda <ikedamsh@nttdata.co.jp>
Date: Tue, 28 Apr 2020 01:39:25 +0000
Subject: [PATCH] Fix to report wait events about timeline history file.
Even though a timeline history file is read or written,
some wait events are not reported. This patch fixes those issues.
---
src/backend/access/transam/timeline.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.cindex de57d69..7a652a8 100644--- a/src/backend/access/transam/timeline.c+++ b/src/backend/access/transam/timeline.c@@ -123,6 +123,7 @@ readTimeLineHistory(TimeLineID targetTLI)
* Parse the file...
*/
prevend = InvalidXLogRecPtr;
+ pgstat_report_wait_start(WAIT_EVENT_TIMELINE_HISTORY_READ);
while (fgets(fline, sizeof(fline), fd) != NULL)
{
/* skip leading whitespace and check for # comment */
@@ -172,6 +173,7 @@ readTimeLineHistory(TimeLineID targetTLI)
/* we ignore the remainder of each line */
}
+ pgstat_report_wait_end();
FreeFile(fd);
@@ -393,6 +395,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
nbytes = strlen(buffer);
errno = 0;
+ pgstat_report_wait_start(WAIT_EVENT_TIMELINE_HISTORY_WRITE);
if ((int) write(fd, buffer, nbytes) != nbytes)
{
int save_errno = errno;
@@ -408,6 +411,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
(errcode_for_file_access(),
errmsg("could not write to file \"%s\": %m", tmppath)));
}
+ pgstat_report_wait_end();
pgstat_report_wait_start(WAIT_EVENT_TIMELINE_HISTORY_SYNC);
if (pg_fsync(fd) != 0)
--
2.9.5
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ikedamsh@oss.nttdata.com, masao.fujii@oss.nttdata.com
Subject: Re: Why are wait events not reported even though it reads/writes a timeline history file?
In-Reply-To: <3480dda4f5a043d8819de9e8653dc331@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox