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]>
Cc: Robert Haas <[email protected]>
Subject: Re: [PATCH] pg_stat_activity: make slow/hanging authentication more visible
Date: Thu, 12 Sep 2024 08:42:25 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAOYmi+k_ZHg94iVKtfbBGU2y8brox7rpMVY9QU1PzB7c6RaTLQ@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>
<[email protected]>
<CAOYmi+k_ZHg94iVKtfbBGU2y8brox7rpMVY9QU1PzB7c6RaTLQ@mail.gmail.com>
On Wed, Sep 11, 2024 at 02:29:49PM -0700, Jacob Champion wrote:
> On Mon, Sep 9, 2024 at 10:30 PM Michael Paquier <[email protected]> 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)
Hm, yeah. We surely should be careful about the consequences of that.
Setting up twice the structure as the patch proposes is kind of
a weird concept, but it feels to me that we should split that and set
the fields in the pre-auth step and ignore the irrelevant ones, then
complete the rest in a second step. We are going to do that anyway if
we want to be able to have backend entries earlier in the
authentication phase.
>> 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.)
Thanks. Robert is arguing upthread about more granularity, which is
also what I understand is the original intention of the wait events.
Noah has a different view. Let's see where it goes but I've given my
opinion.
> 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.
Perhaps that would be useful, not sure. Based on my first
impressions, I'd tend to say no to these extra test cycles, but I'm
okay to be proved wrong, as well.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (5+ messages)
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], [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