On Tue, Sep 03, 2024 at 02:47:57PM -0700, Jacob Champion wrote: > On Sun, Sep 1, 2024 at 5:10 PM Michael Paquier <mich...@paquier.xyz> wrote: >> that gets also used by pgstat_bestart() in >> the case of the patch where !pre_auth? > > To clarify, do you want me to just add the new boolean directly to > pgstat_bestart()'s parameter list?
No. My question was about splitting pgstat_bestart() and pgstat_bestart_pre_auth() in a cleaner way, because authenticated connections finish by calling both, meaning that we do twice the same setup for backend entries depending on the authentication path taken. That seems like a waste. >> The addition of the new wait event states in 0004 is a good idea, >> indeed, > > Thanks! Any thoughts on the two open questions for it?: > 1) Should we add a new wait event class rather than reusing IPC? A new category would be more adapted. IPC is not adapted because are not waiting for another server process. Perhaps just use a new "Authentication" class, as in "The server is waiting for an authentication operation to complete"? > 2) Is the level at which I've inserted calls to > pgstat_report_wait_start()/_end() sane and maintainable? These don't worry me. You are adding twelve event points with only 5 new wait names. Couldn't it be better to have a one-one mapping instead, adding twelve entries in wait_event_names.txt? I am not really on board with the test based on injection points proposed, though. It checks that the "authenticating" flag is set in pg_stat_activity, but it does nothing else. That seems limited. Or are you planning for more? -- Michael
signature.asc
Description: PGP signature
