From: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
To: Michael Paquier <michael@paquier.xyz>
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 17:42:32 +0900
Message-ID: <24cc681e7048f984e490f5d01f7ead19@oss.nttdata.com> (raw)
In-Reply-To: <20200428060958.GG279958@paquier.xyz>
References: <d11b0c910b63684424e06772eb844ab5@oss.nttdata.com>
<c2bed73d-2044-8ab1-075a-75ce0316a814@oss.nttdata.com>
<3480dda4f5a043d8819de9e8653dc331@oss.nttdata.com>
<b208e403-e654-b609-3802-7998b21d016e@oss.nttdata.com>
<20200428060958.GG279958@paquier.xyz>
On 2020-04-28 15:09, Michael Paquier wrote:
> On Tue, Apr 28, 2020 at 02:49:00PM +0900, Fujii Masao wrote:
>> Isn't it safer to report the wait event during fgets() rather than
>> putting
>> those calls around the whole loop, like other code does? For example,
>> writeTimeLineHistory() reports the wait event during read() rather
>> than
>> whole loop.
>
> Yeah, I/O wait events should be taken only during the duration of the
> system calls. Particularly here, you may finish with an elog() that
> causes the wait event to be set longer than it should, leading to a
> rather incorrect state if a snapshot of pg_stat_activity is taken.
> --
Thanks for your comments.
I fixed it to report the wait event during fgets() only.
Please review the v2 patch I attached.
Regard,
--
Masahiro Ikeda
Attachments:
[text/x-diff] v2-0001-Fix-to-report-wait-events-about-timeline-history-.patch (1.7K, ../24cc681e7048f984e490f5d01f7ead19@oss.nttdata.com/2-v2-0001-Fix-to-report-wait-events-about-timeline-history-.patch)
download | inline diff:
From 20b6eb5bb7af574e4395f653917db5a54d13d7d5 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 v2] 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 | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.cindex de57d69..3e1b90f 100644--- a/src/backend/access/transam/timeline.c+++ b/src/backend/access/transam/timeline.c@@ -123,8 +123,15 @@ readTimeLineHistory(TimeLineID targetTLI)
* Parse the file...
*/
prevend = InvalidXLogRecPtr;
- while (fgets(fline, sizeof(fline), fd) != NULL)+ for (;;)
{
+ char *result;+ pgstat_report_wait_start(WAIT_EVENT_TIMELINE_HISTORY_READ);+ result = fgets(fline, sizeof(fline), fd);+ pgstat_report_wait_end();+ if (result == NULL)+ break;+
/* skip leading whitespace and check for # comment */
char *ptr;
TimeLineID tli;
@@ -393,6 +400,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 +416,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, michael@paquier.xyz, 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: <24cc681e7048f984e490f5d01f7ead19@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