MIME-Version: 1.0
References: <CA+Tgmoade_mRdZQHMub31Y9YKg5FpZzwvzYP35R8AVszoRGWcg@mail.gmail.com>
 <20240910205850.5d.nmisch@google.com> <CA+Tgmoa9t-ei=mNjyjdcXxQOY=r0PO-+ncHpryjLaFgV9Y0tyg@mail.gmail.com>
 <20240913145621.43.nmisch@google.com> <CAOYmi+kLzSWrDHZbJg8bWZ94oP_K98mkoEvetgupOBVoy5H_ag@mail.gmail.com>
 <ZysDL6JAGR7UyCA9@paquier.xyz> <Zyww1XGpE1hQa0s1@paquier.xyz>
 <CAOYmi+k7Mz5RmaeHKTEcOPSXBLzATD=ofOXS+JpHB8=-cNHR0Q@mail.gmail.com>
 <faesruozcdcg2aksscb3dcojy4gqjqsyaxfajkqzm4kozjowgm@nmjgrrvdax25>
 <CAOYmi+kUkCkSV13-a0b0rwmtge_uZnPKQx-yLFQJdfoceWFH2g@mail.gmail.com> <wujfqghd2uucboqjgo2igtsedrfp7mbif7iavu52asvfr72fju@27jevcaoz555>
In-Reply-To: <wujfqghd2uucboqjgo2igtsedrfp7mbif7iavu52asvfr72fju@27jevcaoz555>
From: Jacob Champion <jacob.champion@enterprisedb.com>
Date: Thu, 7 Nov 2024 12:11:46 -0800
Message-ID: <CAOYmi+neRXD5LKUthKDaGLfAzApoyvL4OJF-VffifdwyqTdxsQ@mail.gmail.com>
Subject: Re: [PATCH] pg_stat_activity: make slow/hanging authentication more visible
To: Andres Freund <andres@anarazel.de>
Cc: Michael Paquier <michael@paquier.xyz>, Robert Haas <robertmhaas@gmail.com>, 
	Noah Misch <noah@leadboat.com>, Andrew Dunstan <andrew@dunslane.net>, 
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>, 
	Euler Taveira <euler.taveira@enterprisedb.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAOYmi%2BneRXD5LKUthKDaGLfAzApoyvL4OJF-VffifdwyqTdxsQ%40mail.gmail.com>
Precedence: bulk

On Thu, Nov 7, 2024 at 11:41=E2=80=AFAM Andres Freund <andres@anarazel.de> =
wrote:
> I think the patch should not be merged as-is. It's just too ugly and frag=
ile.

Understood; I'm trying to find a way forward, and I'm pointing out
that the alternatives I've tried seem to me to be _more_ fragile.

Are there any items in this list that you disagree with/are less
concerned about?

- the pre-auth step must always initialize the entire pgstat struct
- two-step initialization requires two PGSTAT_BEGIN_WRITE_ACTIVITY()
calls for _every_ backend
- two-step initialization requires us to couple against the order that
authentication information is being filled in (pre-auth, post-auth, or
both)

> I think it might make more sense to use pgstat_report_activity() or such =
here,
> rather than using wait events to describe something that's not a wait.

I'm not sure why you're saying these aren't waits. If pam_authenticate
is capable of hanging for hours and bringing down a production system,
is that not a "wait"?

> > I agree that would be amazing! I'm not about to tackle reliable
> > interrupts for all of the current blocking auth code for v18, though.
> > I'm just trying to make it observable when we do something that
> > blocks.
>
> Well, with that justification we could end up adding wait events for larg=
e
> swaths of code that might not actually ever wait.

If it were hypothetically useful to do so, would that be a problem?
I'm trying not to propose things that aren't actually useful.

--Jacob