From: | Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> |
---|---|
To: | Noah Misch <noah(at)leadboat(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Euler Taveira <euler(dot)taveira(at)enterprisedb(dot)com> |
Subject: | Re: [PATCH] pg_stat_activity: make slow/hanging authentication more visible |
Date: | 2024-07-08 21:09:21 |
Message-ID: | CAOYmi+n9tn3xMQk+wqvmBW_4_7xVJQ8DKhHKj4eCYVNnw1=jVA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Jun 30, 2024 at 10:48 AM Noah Misch <noah(at)leadboat(dot)com> wrote:
> That looks like a reasonable user experience. Is any field newly-nullable?
Technically I think the answer is no, since backends such as walwriter
already have null database and user fields. It's new for a client
backend to have nulls there, though.
> That said, it
> may be more fruitful to arrange for authentication timeout to cut through PAM
> etc.
That seems mostly out of our hands -- the misbehaving modules are free
to ignore our signals (and do). Is there another way to force the
issue?
> Hanging connection slots hurt even if they lack an xmin.
Oh, would releasing the xmin not really move the needle, then?
> I assume it
> takes an immediate shutdown to fix them?
That's my understanding, yeah.
> > Would anyone like me to be more aggressive, and create a pgstat entry
> > as soon as we have the opening transaction? Or... as soon as a
> > connection is made?
>
> All else being equal, I'd like backends to have one before taking any lmgr
> lock or snapshot.
I can look at this for the next patchset version.
> > I haven't decided how to test these patches. Seems like a potential
> > use case for injection points, but I think I'd need to preload an
> > injection library rather than using the existing extension. Does that
> > seem like an okay way to go?
>
> Yes.
I misunderstood how injection points worked. No preload module needed,
so v2 adds a waitpoint and a test along with a couple of needed tweaks
to BackgroundPsql. I think 0001 should probably be applied
independently.
Thanks,
--Jacob
Attachment | Content-Type | Size |
---|---|---|
v2-0001-BackgroundPsql-handle-empty-query-results.patch | application/octet-stream | 2.6 KB |
v2-0002-Test-Cluster-let-background_psql-work-asynchronou.patch | application/octet-stream | 3.1 KB |
v2-0004-WIP-report-external-auth-calls-as-wait-events.patch | application/octet-stream | 7.5 KB |
v2-0003-pgstat-report-in-earlier-with-STATE_AUTHENTICATIN.patch | application/octet-stream | 8.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Dunstan | 2024-07-08 21:18:14 | Re: 010_pg_basebackup.pl vs multiple filesystems |
Previous Message | Andrew Dunstan | 2024-07-08 20:56:10 | Re: tests fail on windows with default git settings |