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

[Jacob Champion]

On Tue, Nov 5, 2024 at 9:48 PM Michael Paquier <mich...@paquier.xyz> wrote:
> +PAM_ACCT_MGMT  "Waiting for the local PAM service to validate the user 
> account."
> +PAM_AUTHENTICATE       "Waiting for the local PAM service to authenticate 
> the user."
>
> Is "local" required for both?  Perhaps just use "the PAM service".

Done in v5.

> +SSPI_LOOKUP_ACCOUNT_SID        "Waiting for Windows to find the user's 
> account SID."
>
> We don't document SID in doc/.  So perhaps this should add be "SID
> (system identifier)".

I switched to "user's security identifier", which seems to be
search-engine-friendly.

On Wed, Nov 6, 2024 at 7:15 PM Michael Paquier <mich...@paquier.xyz> wrote:
> 0002 has been done as ba08edb06545 after adding a bit more
> documentation that was missing.  0001 as well with 70291a3c66ec.

Thanks!

> Note that 0003 is lacking an EXTRA_INSTALL in the Makefile of
> src/test/authentication/, or the test would fail if doing for example
> a `make check` in this path.
>
> The following nit is also required in the script for installcheck, to
> skip the test if the module is not installed:
> if (!$node->check_extension('injection_points'))
> {
>     plan skip_all => 'Extension injection_points not installed';
> }

Fixed.

> 007_injection_points.pl is a name too generic as it could apply in a
> lot more places, without being linked to injection points.  How about
> something like 007_pre_auth.pl?

Renamed.

Thanks!
--Jacob

1:  64289b97e5 < -:  ---------- BackgroundPsql: handle empty query results
2:  18a9531a25 < -:  ---------- Test::Cluster: let background_psql() work 
asynchronously
3:  c8071f91d8 ! 1:  e755fdccf1 pgstat: report in earlier with STATE_STARTING
    @@ src/test/authentication/Makefile: subdir = src/test/authentication
      top_builddir = ../../..
      include $(top_builddir)/src/Makefile.global
      
    ++EXTRA_INSTALL = src/test/modules/injection_points
    ++
     +export enable_injection_points
     +
      check:
    @@ src/test/authentication/meson.build: tests += {
            't/004_file_inclusion.pl',
            't/005_sspi.pl',
            't/006_login_trigger.pl',
    -+      't/007_injection_points.pl',
    ++      't/007_pre_auth.pl',
          ],
        },
      }
     
    - ## src/test/authentication/t/007_injection_points.pl (new) ##
    + ## src/test/authentication/t/007_pre_auth.pl (new) ##
     @@
     +
     +# Copyright (c) 2021-2024, PostgreSQL Global Development Group
     +
    -+# Tests requiring injection_points functionality, to check on behavior 
that
    -+# would otherwise race against authentication.
    ++# Tests for connection behavior prior to authentication.
     +
     +use strict;
     +use warnings FATAL => 'all';
    @@ src/test/authentication/t/007_injection_points.pl (new)
     +]);
     +
     +$node->start;
    ++
    ++# Check if the extension injection_points is available, as it may be
    ++# possible that this script is run with installcheck, where the module
    ++# would not be installed by default.
    ++if (!$node->check_extension('injection_points'))
    ++{
    ++  plan skip_all => 'Extension injection_points not installed';
    ++}
    ++
     +$node->safe_psql('postgres', 'CREATE EXTENSION injection_points');
     +
     +# Connect to the server and inject a waitpoint.
4:  d14b97cb77 ! 2:  858e95f996 Report external auth calls as wait events
    @@ src/backend/utils/activity/wait_event_names.txt: XACT_GROUP_UPDATE       
"Waiting for
     +LDAP_BIND_FOR_SEARCH      "Waiting for an LDAP bind operation to search 
the directory."
     +LDAP_INITIALIZE   "Waiting to initialize an LDAP connection."
     +LDAP_SEARCH       "Waiting for an LDAP search operation to complete."
    -+PAM_ACCT_MGMT     "Waiting for the local PAM service to validate the user 
account."
    -+PAM_AUTHENTICATE  "Waiting for the local PAM service to authenticate the 
user."
    ++PAM_ACCT_MGMT     "Waiting for the PAM service to validate the user 
account."
    ++PAM_AUTHENTICATE  "Waiting for the PAM service to authenticate the user."
     +RADIUS_TRANSACTION        "Waiting for a RADIUS transaction to complete."
     +SSPI_ACCEPT_SECURITY_CONTEXT      "Waiting for a Windows security 
provider to accept the client's SSPI token."
     +SSPI_ACQUIRE_CREDENTIALS_HANDLE   "Waiting for a Windows security 
provider to acquire server credentials for SSPI."
    -+SSPI_LOOKUP_ACCOUNT_SID   "Waiting for Windows to find the user's account 
SID."
    ++SSPI_LOOKUP_ACCOUNT_SID   "Waiting for Windows to find the user's 
security identifier."
     +SSPI_MAKE_UPN     "Waiting for Windows to translate a Kerberos UPN."
     +
     +ABI_compatibility:

v5-0001-pgstat-report-in-earlier-with-STATE_STARTING.patch
Description: Binary data

v5-0002-Report-external-auth-calls-as-wait-events.patch
Description: Binary data