On Thu, Nov 7, 2024 at 4:38 PM Jacob Champion <jacob.champ...@enterprisedb.com> wrote: > Oh... I think that alone is enough to change my mind; I neglected the > effects of that little pgstat_report_appname() stinger...
(Note that application_name is not yet set at the site of the first call, so I think the set-unset-set can't happen after all -- but I didn't realize that before a lot of digging, which is further evidence that I need to simplify...) > I will plumb these down as far as I can. While I work on breaking pgstat_bestart() apart, here is a v6 which pushes down the "coarse" wait events. No changes to 0001 yet. I violated the "one event name per call site" rule with TranslateName(). The call pattern there is "call once to figure out the buffer length, then call again to fill it in", and IMO that didn't deserve differentiation. But if anyone objects, I'm happy to change it (and I'd appreciate some name suggestions in that case). While I was breaking apart the LDAP events, I noticed that ldap_unbind() does a lot more than just dropping the connection, so I've refactored things a bit more in order to wrap all those calls. That is done separately in 0003, which I will fold into 0002 once I have confirmation that it's not controversial to anyone. Thanks! --Jacob
1: e755fdccf1 = 1: e755fdccf1 pgstat: report in earlier with STATE_STARTING
2: 858e95f996 ! 2: 3f14df0308 Report external auth calls as wait events
@@ src/backend/libpq/auth.c: pg_SSPI_recvauth(Port *port)
free(tokenuser);
- if (!port->hba->compat_realm)
- {
-- int status = pg_SSPI_make_upn(accountname,
sizeof(accountname),
--
domainname, sizeof(domainname),
--
port->hba->upn_username);
-+ int status;
+@@ src/backend/libpq/auth.c: pg_SSPI_make_upn(char *accountname,
+ */
+
+ samname = psprintf("%s\\%s", domainname, accountname);
+
-+ pgstat_report_wait_start(WAIT_EVENT_SSPI_MAKE_UPN);
-+ status = pg_SSPI_make_upn(accountname, sizeof(accountname),
-+ domainname,
sizeof(domainname),
-+
port->hba->upn_username);
-+ pgstat_report_wait_end();
++ pgstat_report_wait_start(WAIT_EVENT_SSPI_TRANSLATE_NAME);
+ res = TranslateName(samname, NameSamCompatible, NameUserPrincipal,
+ NULL, &upnamesize);
++ pgstat_report_wait_end();
+
+ if ((!res && GetLastError() != ERROR_INSUFFICIENT_BUFFER)
+ || upnamesize == 0)
+@@ src/backend/libpq/auth.c: pg_SSPI_make_upn(char *accountname,
+ /* upnamesize includes the terminating NUL. */
+ upname = palloc(upnamesize);
+
++ pgstat_report_wait_start(WAIT_EVENT_SSPI_TRANSLATE_NAME);
+ res = TranslateName(samname, NameSamCompatible, NameUserPrincipal,
+ upname, &upnamesize);
++ pgstat_report_wait_end();
- if (status != STATUS_OK)
- /* Error already reported from pg_SSPI_make_upn */
+ pfree(samname);
+ if (res)
@@ src/backend/libpq/auth.c: CheckPAMAuth(Port *port, const char *user,
const char *password)
return STATUS_ERROR;
}
@@ src/backend/libpq/auth.c: CheckPAMAuth(Port *port, const char *user,
const char
if (retval != PAM_SUCCESS)
{
-@@ src/backend/libpq/auth.c: CheckLDAPAuth(Port *port)
- if (passwd == NULL)
- return STATUS_EOF; /* client wouldn't send
password */
+@@ src/backend/libpq/auth.c: InitializeLDAPConnection(Port *port, LDAP
**ldap)
+ }
-- if (InitializeLDAPConnection(port, &ldap) == STATUS_ERROR)
-+ pgstat_report_wait_start(WAIT_EVENT_LDAP_INITIALIZE);
-+ r = InitializeLDAPConnection(port, &ldap);
-+ pgstat_report_wait_end();
+ /* Look up a list of LDAP server hosts and port numbers
*/
+- if (ldap_domain2hostlist(domain, &hostlist))
++ pgstat_report_wait_start(WAIT_EVENT_LDAP_HOST_LOOKUP);
++ r = ldap_domain2hostlist(domain, &hostlist);
++ pgstat_report_wait_end();
+
-+ if (r == STATUS_ERROR)
++ if (r)
+ {
+ ereport(LOG,
+ (errmsg("LDAP authentication
could not find DNS SRV records for \"%s\"",
+@@ src/backend/libpq/auth.c: InitializeLDAPConnection(Port *port, LDAP
**ldap)
+
+ if (port->hba->ldaptls)
{
- /* Error message already sent */
- pfree(passwd);
++ pgstat_report_wait_start(WAIT_EVENT_LDAP_START_TLS);
+ #ifndef WIN32
+- if ((r = ldap_start_tls_s(*ldap, NULL, NULL)) != LDAP_SUCCESS)
++ r = ldap_start_tls_s(*ldap, NULL, NULL);
+ #else
+- if ((r = ldap_start_tls_s(*ldap, NULL, NULL, NULL, NULL)) !=
LDAP_SUCCESS)
++ r = ldap_start_tls_s(*ldap, NULL, NULL, NULL, NULL);
+ #endif
++ pgstat_report_wait_end();
++
++ if (r != LDAP_SUCCESS)
+ {
+ ereport(LOG,
+ (errmsg("could not start LDAP TLS
session: %s",
@@ src/backend/libpq/auth.c: CheckLDAPAuth(Port *port)
* Bind with a pre-defined username/password (if available) for
* searching. If none is specified, this turns into an
anonymous bind.
@@ src/backend/libpq/auth.c: CheckLDAPAuth(Port *port)
if (r != LDAP_SUCCESS)
{
-@@ src/backend/libpq/auth.c: CheckRADIUSAuth(Port *port)
- identifiers = list_head(port->hba->radiusidentifiers);
- foreach(server, port->hba->radiusservers)
- {
-- int ret =
PerformRadiusTransaction(lfirst(server),
--
lfirst(secrets),
--
radiusports ? lfirst(radiusports) : NULL,
--
identifiers ? lfirst(identifiers) : NULL,
--
port->user_name,
--
passwd);
-+ int ret;
+@@ src/backend/libpq/auth.c: PerformRadiusTransaction(const char *server,
const char *secret, const char *por
+ return STATUS_ERROR;
+ }
+
+- if (sendto(sock, radius_buffer, packetlength, 0,
+- serveraddrs[0].ai_addr, serveraddrs[0].ai_addrlen) <
0)
++ pgstat_report_wait_start(WAIT_EVENT_RADIUS_SENDTO);
++ r = sendto(sock, radius_buffer, packetlength, 0,
++ serveraddrs[0].ai_addr, serveraddrs[0].ai_addrlen);
++ pgstat_report_wait_end();
+
-+ pgstat_report_wait_start(WAIT_EVENT_RADIUS_TRANSACTION);
-+ ret = PerformRadiusTransaction(lfirst(server),
-+
lfirst(secrets),
-+
radiusports ? lfirst(radiusports) : NULL,
-+
identifiers ? lfirst(identifiers) : NULL,
-+
port->user_name,
-+
passwd);
++ if (r < 0)
+ {
+ ereport(LOG,
+ (errmsg("could not send RADIUS packet: %m")));
+@@ src/backend/libpq/auth.c: PerformRadiusTransaction(const char *server,
const char *secret, const char *por
+ FD_ZERO(&fdset);
+ FD_SET(sock, &fdset);
+
++ pgstat_report_wait_start(WAIT_EVENT_RADIUS_WAIT);
+ r = select(sock + 1, &fdset, NULL, NULL, &timeout);
+ pgstat_report_wait_end();
++
+ if (r < 0)
+ {
+ if (errno == EINTR)
+@@ src/backend/libpq/auth.c: PerformRadiusTransaction(const char *server,
const char *secret, const char *por
+ */
- /*------
- * STATUS_OK = Login OK
+ addrsize = sizeof(remoteaddr);
++
++ pgstat_report_wait_start(WAIT_EVENT_RADIUS_RECVFROM);
+ packetlength = recvfrom(sock, receive_buffer,
RADIUS_BUFFER_SIZE, 0,
+ (struct
sockaddr *) &remoteaddr, &addrsize);
++ pgstat_report_wait_end();
++
+ if (packetlength < 0)
+ {
+ ereport(LOG,
## src/backend/utils/activity/wait_event.c ##
@@ src/backend/utils/activity/wait_event.c: static const char
*pgstat_get_wait_client(WaitEventClient w);
@@ src/backend/utils/activity/wait_event_names.txt: XACT_GROUP_UPDATE
"Waiting for
+GSSAPI_ACCEPT_SEC_CONTEXT "Waiting for a response from a Kerberos server
via GSSAPI."
+LDAP_BIND "Waiting for an LDAP bind operation to authenticate the user."
+LDAP_BIND_FOR_SEARCH "Waiting for an LDAP bind operation to search
the directory."
-+LDAP_INITIALIZE "Waiting to initialize an LDAP connection."
++LDAP_HOST_LOOKUP "Waiting for DNS lookup of LDAP servers."
+LDAP_SEARCH "Waiting for an LDAP search operation to complete."
++LDAP_START_TLS "Waiting for an LDAP StartTLS exchange."
+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."
++RADIUS_RECVFROM "Waiting for a <function>recvfrom</function> call
during a RADIUS transaction."
++RADIUS_SENDTO "Waiting for a <function>sendto</function> call during
a RADIUS transaction."
++RADIUS_WAIT "Waiting for a RADIUS server to respond."
+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
security identifier."
-+SSPI_MAKE_UPN "Waiting for Windows to translate a Kerberos UPN."
++SSPI_TRANSLATE_NAME "Waiting for Windows to translate a Kerberos
UPN."
+
+ABI_compatibility:
+
-: ---------- > 3: fb21328568 squash! Report external auth calls as wait
events
v6-0001-pgstat-report-in-earlier-with-STATE_STARTING.patch
Description: Binary data
v6-0002-Report-external-auth-calls-as-wait-events.patch
Description: Binary data
v6-0003-squash-Report-external-auth-calls-as-wait-events.patch
Description: Binary data
