On Mon, Sep 9, 2024 at 10:30 PM Michael Paquier <mich...@paquier.xyz> wrote: > 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.
I can try to separate them out. I'm a little wary of messing with the CRITICAL_SECTION guarantees, though. I thought the idea was that you filled in the entire struct to prevent tearing. (If I've misunderstood that, please let me know :D) > Perhaps just use a new > "Authentication" class, as in "The server is waiting for an > authentication operation to complete"? Sounds good. > Couldn't it be better to have a one-one mapping > instead, adding twelve entries in wait_event_names.txt? (I have no strong opinions on this myself, but while the debate is ongoing, I'll work on a version of the patch with more detailed wait events. It's easy to collapse them again if that gets the most votes.) > 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? I can test for specific contents of the entry, if you'd like. My primary goal was to test that an entry shows up if that part of the code hangs. I think a regression would otherwise go completely unnoticed. Thanks! --Jacob
