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
From 20b6eb5bb7af574e4395f653917db5a54d13d7d5 Mon Sep 17 00:00:00 2001
From: Masahiro Ikeda <ikeda...@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.c
index 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
