On Thu, Mar 23, 2023 at 8:10 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > Yeah, commit [1] removed the last trace of it. I wonder if we can add > a WAIT_EVENT_SLRU_FLUSH_SYNC wait event in SlruSyncFileTag(), similar > to mdsyncfiletag. This way, we would have covered all sync_syncfiletag > fsyncs with wait events.
Ahh, right. Thanks. The mistake was indeed that SlruSyncFileTag failed to report it while running pg_fsync(). > > In case it's useful again, here's how I noticed: > > > > for X in ` grep WAIT_EVENT_ src/include/utils/wait_event.h | > > sed '/^#/d;s/,//;s/ = .*//' ` > > do > > if ! ( git grep $X | > > grep -v src/include/utils/wait_event.h | > > grep -v src/backend/utils/activity/wait_event.c | > > grep $X > /dev/null ) > > then > > echo "$X is not used" > > fi > > done > > Interesting. It might be an overkill to think of placing it as a > compile-time script to catch similar miss-outs in future. Meh. Parsing C programs from shell scripts is fun for one-off throw-away usage, but I think if we want proper automation here we should look into a way to define wait events in a central file similar to what we do for src/backend/storage/lmgr/lwlocknames.txt. It could give the enum name, the display name, and the documentation sentence on one tab-separated line, and we could generate all the rest from that, or something like that? I suspect that downstream/monitoring tools might appreciate the existence of such a file too.
From 13e8d3d10d773a07c5586c6f8efc4c526b5db1d0 Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.munro@gmail.com> Date: Fri, 24 Mar 2023 10:47:36 +1300 Subject: [PATCH] Fix missing WAIT_EVENT_SLRU_FLUSH_SYNC reporting. Commit dee663f7 moved fsync() calls for SLRU files into the checkpointer, but accidentally stopped reporting WAIT_EVENT_SLRU_FLUSH_SYNC as the wait event while in the system call. Fix, so that SlruSyncFileTag() mirrors the way the equivalent wait event for data files is reported by mdsyncfiletag(). Back-patch to 14, where dee663f7 landed. Reported-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://postgr.es/m/CALj2ACVY3eA261mUDO8xcMJGDwj_OHpaxEUMc5L9jxkj3fvcpg%40mail.gmail.com --- src/backend/access/transam/slru.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c index 5ab86238a9..2a42f31ec2 100644 --- a/src/backend/access/transam/slru.c +++ b/src/backend/access/transam/slru.c @@ -1603,7 +1603,9 @@ SlruSyncFileTag(SlruCtl ctl, const FileTag *ftag, char *path) if (fd < 0) return -1; + pgstat_report_wait_start(WAIT_EVENT_SLRU_FLUSH_SYNC); result = pg_fsync(fd); + pgstat_report_wait_end(); save_errno = errno; CloseTransientFile(fd); -- 2.39.2
