public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Jacob Champion <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: Noah Misch <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Euler Taveira <[email protected]>
Subject: Re: [PATCH] pg_stat_activity: make slow/hanging authentication more visible
Date: Tue, 10 Sep 2024 14:29:57 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAOYmi+n4RhhoaduZ-ndZGR_=kkzUjnaZtzi8g2GFsqt==Hhopg@mail.gmail.com>
References: <CAOYmi+=60deN20WDyCoHCiecgivJxr=98s7s7-C8SkXwrCfHXg@mail.gmail.com>
<[email protected]>
<CAOYmi+=1Y4HKATADR=WYOMhxCkRQpoxH2rY0t1An6A5su1Cw-w@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAOYmi+n4RhhoaduZ-ndZGR_=kkzUjnaZtzi8g2GFsqt==Hhopg@mail.gmail.com>
On Tue, Sep 03, 2024 at 02:47:57PM -0700, Jacob Champion wrote:
> On Sun, Sep 1, 2024 at 5:10 PM Michael Paquier <[email protected]> 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
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (5+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PATCH] pg_stat_activity: make slow/hanging authentication more visible
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox