Re: [PATCH] pg_stat_activity: make slow/hanging authentication more visible

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

In response to

Responses

Browse pgsql-hackers by date

  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