Thread
Commits
-
Re-add tracking of wait event SLRUFlushSync
- aeb6f4b3b0d0 14.8 landed
- 1ed1b84bdcd2 15.3 landed
- 84cc1426745b 16.0 landed
-
Defer flushing of SLRU files.
- dee663f78439 14.0 cited
-
Orphaned wait event
Thomas Munro <thomas.munro@gmail.com> — 2023-03-23T02:12:21Z
Hi, Commit dee663f7 made WAIT_EVENT_SLRU_FLUSH_SYNC redundant, so here's a patch to remove it. 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 -
Re: Orphaned wait event
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2023-03-23T07:10:34Z
On Thu, Mar 23, 2023 at 7:43 AM Thomas Munro <thomas.munro@gmail.com> wrote: > > Hi, > > Commit dee663f7 made WAIT_EVENT_SLRU_FLUSH_SYNC redundant, so here's a > patch to remove it. 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. > 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. [1] commit dee663f7843902535a15ae366cede8b4089f1144 Author: Thomas Munro <tmunro@postgresql.org> Date: Fri Sep 25 18:49:43 2020 +1200 Defer flushing of SLRU files. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com -
Re: Orphaned wait event
Thomas Munro <thomas.munro@gmail.com> — 2023-03-23T22:00:40Z
On Thu, Mar 23, 2023 at 8:10 PM Bharath Rupireddy <bharath.rupireddyforpostgres@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.
-
Re: Orphaned wait event
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2023-03-24T06:23:20Z
Hi, On 3/23/23 11:00 PM, Thomas Munro wrote: > 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. Yeah, I think that makes sense. I'll look at this and start a new thread once I've a patch to share. FWIW, I'm also working on wait event "improvements" (mainly adding extra info per wait event) that 1) I'll share once ready 2) could also probably benefit from your proposal here. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
-
Re: Orphaned wait event
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2023-03-24T06:30:13Z
On Fri, Mar 24, 2023 at 3:31 AM Thomas Munro <thomas.munro@gmail.com> wrote: > > On Thu, Mar 23, 2023 at 8:10 PM Bharath Rupireddy > <bharath.rupireddyforpostgres@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(). Thanks. The attached patch looks good to me. > > > 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. +1. So, with that approach, both wait_event.h and wait_event.c will be auto-generated I believe. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
-
Re: Orphaned wait event
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2023-04-25T15:54:58Z
On Fri, Mar 24, 2023 at 12:00 PM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote: > > On Fri, Mar 24, 2023 at 3:31 AM Thomas Munro <thomas.munro@gmail.com> wrote: > > > > On Thu, Mar 23, 2023 at 8:10 PM Bharath Rupireddy > > <bharath.rupireddyforpostgres@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(). > > Thanks. The attached patch looks good to me. It looks like this patch attached upthread at [1] isn't in yet, meaning WAIT_EVENT_SLRU_FLUSH_SYNC stays unused. IMO, it's worth pushing it to the PG16 branch. It will help add a wait event for SLRU page flushes. Thoughts? [1] https://www.postgresql.org/message-id/CA%2BhUKG%2BewEpxm%3DhPNXyupRUB_SKGh-6tO86viaco0g-P_pm_Cw%40mail.gmail.com -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
-
Re: Orphaned wait event
Michael Paquier <michael@paquier.xyz> — 2023-04-25T21:34:13Z
On Tue, Apr 25, 2023 at 09:24:58PM +0530, Bharath Rupireddy wrote: > It looks like this patch attached upthread at [1] isn't in yet, > meaning WAIT_EVENT_SLRU_FLUSH_SYNC stays unused. IMO, it's worth > pushing it to the PG16 branch. It will help add a wait event for SLRU > page flushes. Thoughts? > > [1] https://www.postgresql.org/message-id/CA%2BhUKG%2BewEpxm%3DhPNXyupRUB_SKGh-6tO86viaco0g-P_pm_Cw%40mail.gmail.com There could be the argument that some external code could abuse of this value for its own needs, but I don't really buy that. I'll go clean up that.. -- Michael