From d122170f942932f9d747f8437a8089990665457a Mon Sep 17 00:00:00 2001 From: Michael Zhilin Date: Thu, 10 Mar 2022 17:23:11 +0300 Subject: [PATCH 2/2] wip: pg_stat_activity: allocate memory differently This patch changes the way memory is allocated for temporary (aka local state) structures. In the case of large values of max_connections (over 1000), memory was allocated to accommodate the worst-case number of active backends which may be wasteful and cause increased latency. The patch makes pg_stat_activity view a bit more efficient by allocating memory as needed for active backends. It's still allocated in chunks. Co-authored-by: Yura Sokolov --- src/backend/utils/activity/backend_status.c | 107 ++++++++++++++------ src/backend/utils/adt/pgstatfuncs.c | 4 +- src/include/utils/backend_status.h | 2 +- 3 files changed, 77 insertions(+), 36 deletions(-) diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index 64567ff5cd7..9fbe8a601db 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -25,6 +25,7 @@ #include "utils/guc.h" /* for application_name */ #include "utils/memutils.h" +#define UINT32_ACCESS_ONCE(var) ((uint32)(*((volatile uint32 *)&(var)))) /* ---------- * Total number of backends including auxiliary @@ -716,6 +717,24 @@ pgstat_report_xact_timestamp(TimestampTz tstamp) PGSTAT_END_WRITE_ACTIVITY(beentry); } +#define PGSTAT_LOCALDATA_CHUNK 128 +#ifdef HAVE_TYPEOF +#define PGSTAT_LOCALDATA_ALLOC(x,cnt) \ + if (x == NULL || x > x##_thr) { \ + x = (typeof(x)) \ + MemoryContextAlloc(backendStatusSnapContext, \ + cnt * sizeof(x[0]) * PGSTAT_LOCALDATA_CHUNK); \ + x##_thr = x + cnt * (PGSTAT_LOCALDATA_CHUNK - 1); \ + } +#else +#define PGSTAT_LOCALDATA_ALLOC(x,cnt) \ + if (x == NULL || x > x##_thr) { \ + x = MemoryContextAlloc(backendStatusSnapContext, \ + cnt * sizeof(x[0]) * PGSTAT_LOCALDATA_CHUNK); \ + x##_thr = x + cnt * (PGSTAT_LOCALDATA_CHUNK - 1); \ + } +#endif + /* ---------- * pgstat_read_current_status() - * @@ -729,16 +748,24 @@ pgstat_read_current_status(void) volatile PgBackendStatus *beentry; LocalPgBackendStatus *localtable; LocalPgBackendStatus *localentry; + PgBackendStatus *localbeentry, + *localbeentry_thr; char *localappname, + *localappname_thr, *localclienthostname, - *localactivity; + *localclienthostname_thr, + *localactivity, + *localactivity_thr; #ifdef USE_SSL - PgBackendSSLStatus *localsslstatus; + PgBackendSSLStatus *localsslstatus, + *localsslstatus_thr; #endif #ifdef ENABLE_GSS - PgBackendGSSStatus *localgssstatus; + PgBackendGSSStatus *localgssstatus, + *localgssstatus_thr; #endif int i; + int pid; if (localBackendStatusTable) return; /* already done */ @@ -756,32 +783,39 @@ pgstat_read_current_status(void) localtable = (LocalPgBackendStatus *) MemoryContextAlloc(backendStatusSnapContext, sizeof(LocalPgBackendStatus) * NumBackendStatSlots); - localappname = (char *) - MemoryContextAlloc(backendStatusSnapContext, - NAMEDATALEN * NumBackendStatSlots); - localclienthostname = (char *) - MemoryContextAlloc(backendStatusSnapContext, - NAMEDATALEN * NumBackendStatSlots); - localactivity = (char *) - MemoryContextAllocHuge(backendStatusSnapContext, - pgstat_track_activity_query_size * NumBackendStatSlots); + #ifdef USE_SSL - localsslstatus = (PgBackendSSLStatus *) - MemoryContextAlloc(backendStatusSnapContext, - sizeof(PgBackendSSLStatus) * NumBackendStatSlots); + localsslstatus = NULL; #endif #ifdef ENABLE_GSS - localgssstatus = (PgBackendGSSStatus *) - MemoryContextAlloc(backendStatusSnapContext, - sizeof(PgBackendGSSStatus) * NumBackendStatSlots); + localgssstatus = NULL; #endif localNumBackends = 0; + localbeentry = NULL; + localappname = NULL; + localclienthostname = NULL; + localactivity = NULL; beentry = BackendStatusArray; localentry = localtable; for (i = 1; i <= NumBackendStatSlots; i++) { + /* + * Check available space in buffers for local state data + */ + PGSTAT_LOCALDATA_ALLOC(localbeentry, 1); + PGSTAT_LOCALDATA_ALLOC(localappname, NAMEDATALEN); + PGSTAT_LOCALDATA_ALLOC(localclienthostname, NAMEDATALEN); + PGSTAT_LOCALDATA_ALLOC(localactivity, pgstat_track_activity_query_size); +#ifdef USE_SSL + PGSTAT_LOCALDATA_ALLOC(localsslstatus, 1); +#endif +#ifdef ENABLE_GSS + PGSTAT_LOCALDATA_ALLOC(localgssstatus, 1); +#endif + + localentry->backendStatus = localbeentry; /* * Follow the protocol of retrying if st_changecount changes while we * copy the entry, or if it's odd. (The check for odd is needed to @@ -796,11 +830,11 @@ pgstat_read_current_status(void) pgstat_begin_read_activity(beentry, before_changecount); - localentry->backendStatus.st_procpid = beentry->st_procpid; + localentry->backendStatus->st_procpid = beentry->st_procpid; /* Skip all the data-copying work if entry is not in use */ - if (localentry->backendStatus.st_procpid > 0) + if (localentry->backendStatus->st_procpid > 0) { - memcpy(&localentry->backendStatus, unvolatize(PgBackendStatus *, beentry), sizeof(PgBackendStatus)); + memcpy(localentry->backendStatus, unvolatize(PgBackendStatus *, beentry), sizeof(PgBackendStatus)); /* * For each PgBackendStatus field that is a pointer, copy the @@ -811,23 +845,23 @@ pgstat_read_current_status(void) * because there's always a \0 at the end of the buffer. */ strcpy(localappname, (char *) beentry->st_appname); - localentry->backendStatus.st_appname = localappname; + localentry->backendStatus->st_appname = localappname; strcpy(localclienthostname, (char *) beentry->st_clienthostname); - localentry->backendStatus.st_clienthostname = localclienthostname; + localentry->backendStatus->st_clienthostname = localclienthostname; strcpy(localactivity, (char *) beentry->st_activity_raw); - localentry->backendStatus.st_activity_raw = localactivity; + localentry->backendStatus->st_activity_raw = localactivity; #ifdef USE_SSL if (beentry->st_ssl) { memcpy(localsslstatus, beentry->st_sslstatus, sizeof(PgBackendSSLStatus)); - localentry->backendStatus.st_sslstatus = localsslstatus; + localentry->backendStatus->st_sslstatus = localsslstatus; } #endif #ifdef ENABLE_GSS if (beentry->st_gss) { memcpy(localgssstatus, beentry->st_gssstatus, sizeof(PgBackendGSSStatus)); - localentry->backendStatus.st_gssstatus = localgssstatus; + localentry->backendStatus->st_gssstatus = localgssstatus; } #endif } @@ -842,8 +876,9 @@ pgstat_read_current_status(void) CHECK_FOR_INTERRUPTS(); } + pid = localentry->backendStatus->st_procpid; /* Only valid entries get included into the local array */ - if (localentry->backendStatus.st_procpid > 0) + if (pid > 0) { PGPROC *proc = BackendIdGetProc(i); if (proc != NULL) @@ -855,12 +890,17 @@ pgstat_read_current_status(void) * leaves the field as NULL for the leader of a parallel * group. */ - if (leader && leader->pid != localentry->backendStatus.st_procpid) + if (leader && leader->pid != localentry->backendStatus->st_procpid) localentry->leader_pid = leader->pid; else localentry->leader_pid = 0; - localentry->wait_event_info = proc->wait_event_info; + localentry->wait_event_info = UINT32_ACCESS_ONCE(proc->wait_event_info); + } + else + { + localentry->leader_pid = 0; + localentry->wait_event_info = 0; } BackendIdGetTransactionIds(i, @@ -868,9 +908,10 @@ pgstat_read_current_status(void) &localentry->backend_xmin); localentry++; - localappname += NAMEDATALEN; - localclienthostname += NAMEDATALEN; - localactivity += pgstat_track_activity_query_size; + localbeentry++; + localappname += strlen(localappname) + 1; + localclienthostname += strlen(localclienthostname) + 1; + localactivity += strlen(localactivity) + 1; #ifdef USE_SSL localsslstatus++; #endif @@ -1083,7 +1124,7 @@ pgstat_fetch_stat_beentry(int beid) if (beid < 1 || beid > localNumBackends) return NULL; - return &localBackendStatusTable[beid - 1].backendStatus; + return localBackendStatusTable[beid - 1].backendStatus; } diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 82f169a2d8b..d5f2448912b 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -500,7 +500,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS) if (!local_beentry) continue; - beentry = &local_beentry->backendStatus; + beentry = local_beentry->backendStatus; /* * Report values for only those backends which are running the given @@ -581,7 +581,7 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) continue; } - beentry = &local_beentry->backendStatus; + beentry = local_beentry->backendStatus; /* If looking for specific PID, ignore all the others */ if (pid != -1 && beentry->st_procpid != pid) diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h index 8f890911486..0b4b426e3aa 100644 --- a/src/include/utils/backend_status.h +++ b/src/include/utils/backend_status.h @@ -245,7 +245,7 @@ typedef struct LocalPgBackendStatus /* * Local version of the backend status entry. */ - PgBackendStatus backendStatus; + PgBackendStatus *backendStatus; /* * The xid of the current transaction if available, InvalidTransactionId -- 2.17.1