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

Jacob Champion <jacob.champion@enterprisedb.com>

From: Jacob Champion <jacob.champion@enterprisedb.com>

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>

Date: 2024-11-11T23:17:44Z

Lists: pgsql-hackers

Attachments

v7-0001-pgstat-report-in-earlier-with-STATE_STARTING.patch (application/octet-stream) patch v7-0001
v7-0002-Report-external-auth-calls-as-wait-events.patch (application/octet-stream) patch v7-0002
v7-0003-squash-Report-external-auth-calls-as-wait-events.patch (application/octet-stream) patch v7-0003

On Fri, Nov 8, 2024 at 4:23 PM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:
> While I work on breaking pgstat_bestart() apart, here is a v6 which
> pushes down the "coarse" wait events. No changes to 0001 yet.

v7 rewrites 0001 by splitting pgstat_bestart() into three phases.
(0002-3 are unchanged.)

1. pgstat_bestart_initial() reports STATE_STARTING, fills in the early
fields and clears out the rest.
2. pgstat_bestart_security() reports the SSL/GSS status of the
connection. This is only for backends with a valid MyProcPort; they
call it twice.
3. pgstat_bestart_final() fills in the user and database IDs, takes
the entry out of STATE_STARTING, and reports the application_name.

I was wondering if maybe I should fill in application_name before
taking the entry out of STATE_STARTING, in order to establish the rule
that "starting" pgstat entries are always partially filled, and that
DBAs can rely on their full contents once they've proceeded past it.
Thoughts?

I've added machinery to 001_ssltests.pl to make sure we see early
transport security stats prior to user authentication. This overlaps
quite a bit with the new 007_pre_auth.pl, so if we'd rather not have
the latter (as briefly discussed upthread) I can move the rest of it
over.

Thanks,
--Jacob

Commits

Fix race condition in TAP test 007_pre_auth
- e2080261cc8c 18.0 landed
Split pgstat_bestart() into three different routines
- c76db55c9085 18.0 landed
backport: Extend background_psql() to be able to start asynchronously
- 6af51bf05a6a 13.21 landed
- 7c07ab62aeb7 14.18 landed
- c0bc11aebb01 15.13 landed
- 6ab58d506bba 16.9 landed
- 49b6f4a02b23 17.5 landed
backport: Improve handling of empty query results in BackgroundPsql
- 3c562b58c20e 13.21 landed
- 3170aece14b8 14.18 landed
- f4b08ccb4ec8 15.13 landed
- fbfd38662f72 16.9 landed
- 31a242e90c88 17.5 landed
Improve handling of empty query results in BackgroundPsql::query()
- 70291a3c66ec 18.0 landed
Extend Cluster.pm's background_psql() to be able to start asynchronously
- ba08edb06545 18.0 landed
dblink: Replace WAIT_EVENT_EXTENSION with custom wait events
- c789f0f6cc5d 17.0 cited