On Tue, 2025-04-08 at 09:10 +0000, Daniel Gustafsson wrote:
> Add function to get memory context stats for processes
>
> This adds a function for retrieving memory context statistics
> and information from backends as well as auxiliary processes.
> The intended usecase is cluster debugging when under memory
> pressure or unanticipated memory usage characteristics.
>
> Discussion:
> https://postgr.es/m/cah2l28v8mc9hdt8qosj8trmkau_8fm_hks41neo9-6zakuz...@mail.gmail.com
>
> Details
> -------
> https://git.postgresql.org/pg/commitdiff/042a66291b04f473cbc72f95f07438abd75ae3a9
>
> [from the patch:]
> diff --git a/src/include/storage/procsignal.h
> b/src/include/storage/procsignal.h
> index 016dfd9b3f6..cfe14631445 100644
> --- a/src/include/storage/procsignal.h
> +++ b/src/include/storage/procsignal.h
> [...]
> +extern dsa_area *area;
This commit causes problems for PostGIS, because the name "area" collides
with a PostGIS object:
postgis_legacy.c:58:28: error: ‘area’ redeclared as different kind of symbol
58 | POSTGIS_DEPRECATE("3.0.0", area)
| ^~~~
postgis_legacy.c:40:15: note: in definition of macro ‘POSTGIS_DEPRECATE’
40 | Datum funcname(PG_FUNCTION_ARGS); \
| ^~~~~~~~
In file included from ../libpgcommon/lwgeom_pg.h:24,
from postgis_legacy.c:37:
/home/laurenz/pg/include/postgresql/server/utils/memutils.h:403:18: note:
previous declaration of ‘area’ with type ‘dsa_area *’
403 | extern dsa_area *area;
| ^~~~
Now one can take the position that PostGIS as dependent library hs to
adapt, but I think "area" is too generic a name. Could you envision
renaming the global variable to something like "shm_area"?
Attached is a patch for this change.
I am not wedded to the name at all, it was just the first thing that
popped into my head.
Yours,
Laurenz Albe
From 9a207d76d4b1bbe5904bf346c6621324a9dd7359 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <[email protected]>
Date: Thu, 10 Apr 2025 13:38:10 +0200
Subject: [PATCH v1] Rename a global variable to avoid name collisions
"area", introduced by 042a66291b04, is a very generic name, and
indeed it collides with an object defined by PostGIS.
While it is arguably the job of the dependent library to avoid name
collisions, using a generic name like that is just an invitation
for problems of that kind.
Fix by renaming the variable to "shm_area".
---
src/backend/utils/adt/mcxtfuncs.c | 14 +++++------
src/backend/utils/mmgr/mcxt.c | 40 +++++++++++++++----------------
src/include/utils/memutils.h | 2 +-
3 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index 3ede88e5036..154ea38fdc2 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -533,21 +533,21 @@ pg_get_process_memory_contexts(PG_FUNCTION_ARGS)
*/
Assert(memCxtArea->memstats_dsa_handle != DSA_HANDLE_INVALID);
/* Attach to the dsa area if we have not already done so */
- if (area == NULL)
+ if (shm_area == NULL)
{
MemoryContext oldcontext = CurrentMemoryContext;
MemoryContextSwitchTo(TopMemoryContext);
- area = dsa_attach(memCxtArea->memstats_dsa_handle);
+ shm_area = dsa_attach(memCxtArea->memstats_dsa_handle);
MemoryContextSwitchTo(oldcontext);
- dsa_pin_mapping(area);
+ dsa_pin_mapping(shm_area);
}
/*
* Backend has finished publishing the stats, project them.
*/
memcxt_info = (MemoryStatsEntry *)
- dsa_get_address(area, memCxtState[procNumber].memstats_dsa_pointer);
+ dsa_get_address(shm_area, memCxtState[procNumber].memstats_dsa_pointer);
#define PG_GET_PROCESS_MEMORY_CONTEXTS_COLS 12
for (int i = 0; i < memCxtState[procNumber].total_stats; i++)
@@ -566,7 +566,7 @@ pg_get_process_memory_contexts(PG_FUNCTION_ARGS)
if (DsaPointerIsValid(memcxt_info[i].name))
{
- name = (char *) dsa_get_address(area, memcxt_info[i].name);
+ name = (char *) dsa_get_address(shm_area, memcxt_info[i].name);
values[0] = CStringGetTextDatum(name);
}
else
@@ -574,7 +574,7 @@ pg_get_process_memory_contexts(PG_FUNCTION_ARGS)
if (DsaPointerIsValid(memcxt_info[i].ident))
{
- ident = (char *) dsa_get_address(area, memcxt_info[i].ident);
+ ident = (char *) dsa_get_address(shm_area, memcxt_info[i].ident);
values[1] = CStringGetTextDatum(ident);
}
else
@@ -586,7 +586,7 @@ pg_get_process_memory_contexts(PG_FUNCTION_ARGS)
path_datum = (Datum *) palloc(path_length * sizeof(Datum));
if (DsaPointerIsValid(memcxt_info[i].path))
{
- path_int = (int *) dsa_get_address(area, memcxt_info[i].path);
+ path_int = (int *) dsa_get_address(shm_area, memcxt_info[i].path);
for (int j = 0; j < path_length; j++)
path_datum[j] = Int32GetDatum(path_int[j]);
path_array = construct_array_builtin(path_datum, path_length, INT4OID);
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index cf4e22bf1cc..f32ed7967d9 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -172,7 +172,7 @@ MemoryContext CurTransactionContext = NULL;
/* This is a transient link to the active portal's memory context: */
MemoryContext PortalContext = NULL;
-dsa_area *area = NULL;
+dsa_area *shm_area = NULL;
static void MemoryContextDeleteOnly(MemoryContext context);
static void MemoryContextCallResetCallbacks(MemoryContext context);
@@ -1499,19 +1499,19 @@ ProcessGetMemoryContextInterrupt(void)
MemoryContextSwitchTo(TopMemoryContext);
- area = dsa_create(memCxtArea->lw_lock.tranche);
+ shm_area = dsa_create(memCxtArea->lw_lock.tranche);
- handle = dsa_get_handle(area);
+ handle = dsa_get_handle(shm_area);
MemoryContextSwitchTo(oldcontext);
- dsa_pin_mapping(area);
+ dsa_pin_mapping(shm_area);
/*
* Pin the DSA area, this is to make sure the area remains attachable
* even if current backend exits. This is done so that the statistics
* are published even if the process exits while a client is waiting.
*/
- dsa_pin(area);
+ dsa_pin(shm_area);
/* Set the handle in shared memory */
memCxtArea->memstats_dsa_handle = handle;
@@ -1521,14 +1521,14 @@ ProcessGetMemoryContextInterrupt(void)
* If DSA exists, created by another process publishing statistics, attach
* to it.
*/
- else if (area == NULL)
+ else if (shm_area == NULL)
{
MemoryContext oldcontext = CurrentMemoryContext;
MemoryContextSwitchTo(TopMemoryContext);
- area = dsa_attach(memCxtArea->memstats_dsa_handle);
+ shm_area = dsa_attach(memCxtArea->memstats_dsa_handle);
MemoryContextSwitchTo(oldcontext);
- dsa_pin_mapping(area);
+ dsa_pin_mapping(shm_area);
}
LWLockRelease(&memCxtArea->lw_lock);
@@ -1545,7 +1545,7 @@ ProcessGetMemoryContextInterrupt(void)
* Free any previous allocations, free the name, ident and path
* pointers before freeing the pointer that contains them.
*/
- free_memorycontextstate_dsa(area, memCxtState[idx].total_stats,
+ free_memorycontextstate_dsa(shm_area, memCxtState[idx].total_stats,
memCxtState[idx].memstats_dsa_pointer);
}
@@ -1556,10 +1556,10 @@ ProcessGetMemoryContextInterrupt(void)
*/
memCxtState[idx].total_stats = stats_num;
memCxtState[idx].memstats_dsa_pointer =
- dsa_allocate0(area, stats_num * sizeof(MemoryStatsEntry));
+ dsa_allocate0(shm_area, stats_num * sizeof(MemoryStatsEntry));
meminfo = (MemoryStatsEntry *)
- dsa_get_address(area, memCxtState[idx].memstats_dsa_pointer);
+ dsa_get_address(shm_area, memCxtState[idx].memstats_dsa_pointer);
if (summary)
{
@@ -1572,7 +1572,7 @@ ProcessGetMemoryContextInterrupt(void)
&stat, true);
path = lcons_int(1, path);
PublishMemoryContext(meminfo, cxt_id, TopMemoryContext, path, stat,
- 1, area, 100);
+ 1, shm_area, 100);
cxt_id = cxt_id + 1;
/*
@@ -1602,7 +1602,7 @@ ProcessGetMemoryContextInterrupt(void)
*/
memCxtState[idx].total_stats = cxt_id++;
PublishMemoryContext(meminfo, cxt_id, c, path,
- grand_totals, num_contexts, area, 100);
+ grand_totals, num_contexts, shm_area, 100);
}
memCxtState[idx].total_stats = cxt_id;
@@ -1632,7 +1632,7 @@ ProcessGetMemoryContextInterrupt(void)
if (context_id < (max_stats - 1) || stats_count <= max_stats)
{
/* Copy statistics to DSA memory */
- PublishMemoryContext(meminfo, context_id, cur, path, stat, 1, area, 100);
+ PublishMemoryContext(meminfo, context_id, cur, path, stat, 1, shm_area, 100);
}
else
{
@@ -1657,8 +1657,8 @@ ProcessGetMemoryContextInterrupt(void)
int namelen = strlen("Remaining Totals");
num_individual_stats = context_id + 1;
- meminfo[max_stats - 1].name = dsa_allocate(area, namelen + 1);
- nameptr = dsa_get_address(area, meminfo[max_stats - 1].name);
+ meminfo[max_stats - 1].name = dsa_allocate(shm_area, namelen + 1);
+ nameptr = dsa_get_address(shm_area, meminfo[max_stats - 1].name);
strncpy(nameptr, "Remaining Totals", namelen);
meminfo[max_stats - 1].ident = InvalidDsaPointer;
meminfo[max_stats - 1].path = InvalidDsaPointer;
@@ -1921,18 +1921,18 @@ AtProcExit_memstats_cleanup(int code, Datum arg)
}
/* If the dsa mapping could not be found, attach to the area */
- if (area == NULL)
- area = dsa_attach(memCxtArea->memstats_dsa_handle);
+ if (shm_area == NULL)
+ shm_area = dsa_attach(memCxtArea->memstats_dsa_handle);
/*
* Free the memory context statistics, free the name, ident and path
* pointers before freeing the pointer that contains these pointers and
* integer statistics.
*/
- free_memorycontextstate_dsa(area, memCxtState[idx].total_stats,
+ free_memorycontextstate_dsa(shm_area, memCxtState[idx].total_stats,
memCxtState[idx].memstats_dsa_pointer);
- dsa_detach(area);
+ dsa_detach(shm_area);
LWLockRelease(&memCxtState[idx].lw_lock);
}
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index d328270fafc..79fb4302139 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -400,5 +400,5 @@ extern void HandleGetMemoryContextInterrupt(void);
extern Size MemoryContextReportingShmemSize(void);
extern void MemoryContextReportingShmemInit(void);
extern void AtProcExit_memstats_cleanup(int code, Datum arg);
-extern dsa_area *area;
+extern dsa_area *shm_area;
#endif /* MEMUTILS_H */
--
2.49.0
