On Fri, 2022-09-09 at 12:14 -0500, Justin Pryzby wrote: > On Sat, Sep 03, 2022 at 11:40:03PM -0400, Reid Thompson wrote: > > > > + 0, 0, INT_MAX, > > > > + NULL, NULL, NULL > > > I think this needs a maximum like INT_MAX/1024/1024 > > > > Is this noting that we'd set a ceiling of 2048MB? > > The reason is that you're later multiplying it by 1024*1024, so you > need > to limit it to avoid overflowing. Compare with > min_dynamic_shared_memory, Log_RotationSize, maintenance_work_mem, > autovacuum_work_mem.
What I originally attempted to implement is: GUC "max_total_backend_memory" max value as INT_MAX = 2147483647 MB (2251799812636672 bytes). And the other variables and comparisons as bytes represented as uint64 to avoid overflow. Is this invalid? > typo: Explicitely corrected > + errmsg("request will exceed postgresql.conf > defined max_total_backend_memory limit (%lu > %lu)", > > I wouldn't mention postgresql.conf - it could be in > postgresql.auto.conf, or an include file, or a -c parameter. > Suggest: allocation would exceed max_total_backend_memory limit... > updated > > + ereport(LOG, errmsg("decrease reduces reported > backend memory allocated below zero; setting reported to 0")); > > Suggest: deallocation would decrease backend memory below zero; updated > + {"max_total_backend_memory", PGC_SIGHUP, > RESOURCES_MEM, > > > > Should this be PGC_SU_BACKEND to allow a superuser to set a higher > limit (or no limit)? Sounds good to me. I'll update to that. Would PGC_SUSET be too open? > There's compilation warning under mingw cross compile due to > sizeof(long). See d914eb347 and other recent commits which I guess > is > the current way to handle this. > http://cfbot.cputube.org/reid-thompson.html updated %lu to %llu and changed cast from uint64 to unsigned long long in the ereport call > For performance test, you'd want to check what happens with a large > number of max_connections (and maybe a large number of clients). TPS > isn't the only thing that matters. For example, a utility command > might > sometimes do a lot of allocations (or deallocations), or a > "parameterized nested loop" may loop over over many outer tuples and > reset for each. There's also a lot of places that reset to a > "per-tuple" context. I started looking at its performance, but > nothing > to show yet. Thanks > Would you keep people copied on your replies ("reply all") ? > Otherwise > I (at least) may miss them. I think that's what's typical on these > lists (and the list tool is smart enough not to send duplicates to > people who are direct recipients). Ok - will do, thanks. -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com
From 584a04f1b53948049e73165a4ffdd544c950ab0d Mon Sep 17 00:00:00 2001 From: Reid Thompson <jreidthomp...@nc.rr.com> Date: Thu, 11 Aug 2022 12:01:25 -0400 Subject: [PATCH 1/2] Add tracking of backend memory allocated to pg_stat_activity This new field displays the current bytes of memory allocated to the backend process. It is updated as memory for the process is malloc'd/free'd. Memory allocated to items on the freelist is included in the displayed value. Dynamic shared memory allocations are included only in the value displayed for the backend that created them, they are not included in the value for backends that are attached to them to avoid double counting. On occasion, orphaned memory segments may be cleaned up on postmaster startup. This may result in decreasing the sum without a prior increment. We limit the floor of backend_mem_allocated to zero. Updated pg_stat_activity documentation for the new column. --- doc/src/sgml/monitoring.sgml | 12 +++ src/backend/catalog/system_views.sql | 1 + src/backend/storage/ipc/dsm_impl.c | 80 +++++++++++++++ src/backend/utils/activity/backend_status.c | 105 ++++++++++++++++++++ src/backend/utils/adt/pgstatfuncs.c | 4 +- src/backend/utils/mmgr/aset.c | 18 ++++ src/backend/utils/mmgr/generation.c | 15 +++ src/backend/utils/mmgr/slab.c | 21 ++++ src/include/catalog/pg_proc.dat | 6 +- src/include/utils/backend_status.h | 7 +- src/test/regress/expected/rules.out | 9 +- 11 files changed, 269 insertions(+), 9 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 1d9509a2f6..40ae638f25 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -947,6 +947,18 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>backend_mem_allocated</structfield> <type>bigint</type> + </para> + <para> + The byte count of memory allocated to this backend. Dynamic shared memory + allocations are included only in the value displayed for the backend that + created them, they are not included in the value for backends that are + attached to them to avoid double counting. + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>query</structfield> <type>text</type> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 5a844b63a1..d23f0e9dbb 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -863,6 +863,7 @@ CREATE VIEW pg_stat_activity AS S.backend_xid, s.backend_xmin, S.query_id, + S.backend_mem_allocated, S.query, S.backend_type FROM pg_stat_get_activity(NULL) AS S diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c index e1b90c5de4..3356bb65b5 100644 --- a/src/backend/storage/ipc/dsm_impl.c +++ b/src/backend/storage/ipc/dsm_impl.c @@ -66,6 +66,7 @@ #include "postmaster/postmaster.h" #include "storage/dsm_impl.h" #include "storage/fd.h" +#include "utils/backend_status.h" #include "utils/guc.h" #include "utils/memutils.h" @@ -232,6 +233,13 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_backend_mem_allocated_decrease(*mapped_size); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && shm_unlink(name) != 0) @@ -332,6 +340,36 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + { + /* + * Posix creation calls dsm_impl_posix_resize implying that resizing + * occurs or may be added in the future. As implemented + * dsm_impl_posix_resize utilizes fallocate or truncate, passing the + * whole new size as input, growing the allocation as needed (only + * truncate supports shrinking). We update by replacing the old + * allocation with the new. + */ +#if defined(HAVE_POSIX_FALLOCATE) && defined(__linux__) + /* + * posix_fallocate does not shrink allocations, adjust only on + * allocation increase. + */ + if (request_size > *mapped_size) + { + pgstat_report_backend_mem_allocated_decrease(*mapped_size); + pgstat_report_backend_mem_allocated_increase(request_size); + } +#else + pgstat_report_backend_mem_allocated_decrease(*mapped_size); + pgstat_report_backend_mem_allocated_increase(request_size); +#endif + } *mapped_address = address; *mapped_size = request_size; close(fd); @@ -537,6 +575,14 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_backend_mem_allocated_decrease(*mapped_size); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && shmctl(ident, IPC_RMID, NULL) < 0) @@ -584,6 +630,13 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_backend_mem_allocated_increase(request_size); *mapped_address = address; *mapped_size = request_size; @@ -652,6 +705,13 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return false; } + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_backend_mem_allocated_decrease(*mapped_size); *impl_private = NULL; *mapped_address = NULL; *mapped_size = 0; @@ -768,6 +828,12 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return false; } + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_backend_mem_allocated_increase(info.RegionSize); *mapped_address = address; *mapped_size = info.RegionSize; *impl_private = hmap; @@ -812,6 +878,13 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + pgstat_report_backend_mem_allocated_decrease(*mapped_size); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && unlink(name) != 0) @@ -933,6 +1006,13 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_backend_mem_allocated_increase(request_size); *mapped_address = address; *mapped_size = request_size; diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index c7ed1e6d7a..45da3af213 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -49,6 +49,8 @@ int pgstat_track_activity_query_size = 1024; /* exposed so that backend_progress.c can access it */ PgBackendStatus *MyBEEntry = NULL; +/* Memory allocated to this backend prior to pgstats initialization */ +uint64 backend_mem_allocated = 0; static PgBackendStatus *BackendStatusArray = NULL; static char *BackendAppnameBuffer = NULL; @@ -400,6 +402,13 @@ pgstat_bestart(void) lbeentry.st_progress_command_target = InvalidOid; lbeentry.st_query_id = UINT64CONST(0); + /* + * Move sum of memory allocated prior to pgstats initialization to pgstats + * and zero the local variable. + */ + lbeentry.backend_mem_allocated = backend_mem_allocated; + backend_mem_allocated = 0; + /* * we don't zero st_progress_param here to save cycles; nobody should * examine it until st_progress_command has been set to something other @@ -1148,3 +1157,99 @@ pgstat_clip_activity(const char *raw_activity) return activity; } + +/* -------- + * pgstat_report_backend_mem_allocated_increase() - + * + * Called to report increase in memory allocated for this backend + * -------- + */ +void +pgstat_report_backend_mem_allocated_increase(uint64 allocation) +{ + volatile PgBackendStatus *beentry = MyBEEntry; + + if (!beentry || !pgstat_track_activities) + { + /* + * Account for memory before pgstats is initialized. This will be + * migrated to pgstats on initialization. + */ + backend_mem_allocated += allocation; + + return; + } + + /* + * Update my status entry, following the protocol of bumping + * st_changecount before and after. We use a volatile pointer here to + * ensure the compiler doesn't try to get cute. + */ + PGSTAT_BEGIN_WRITE_ACTIVITY(beentry); + beentry->backend_mem_allocated += allocation; + PGSTAT_END_WRITE_ACTIVITY(beentry); +} + +/* -------- + * pgstat_report_backend_mem_allocated_decrease() - + * + * Called to report decrease in memory allocated for this backend + * -------- + */ +void +pgstat_report_backend_mem_allocated_decrease(uint64 deallocation) +{ + volatile PgBackendStatus *beentry = MyBEEntry; + + /* + * Cases may occur where shared memory from a previous postmaster + * invocation still exist. These are cleaned up at startup by + * dsm_cleanup_using_control_segment. Limit decreasing memory allocated to + * zero in case no corresponding prior increase exists or decrease has + * already been accounted for. + */ + + if (!beentry || !pgstat_track_activities) + { + /* + * Account for memory before pgstats is initialized. This will be + * migrated to pgstats on initialization. Do not allow + * backend_mem_allocated to go below zero. If pgstats has not been + * initialized, we are in startup and we set backend_mem_allocated to + * zero in cases where it would go negative and skip generating an + * ereport. + */ + if (deallocation > backend_mem_allocated) + backend_mem_allocated = 0; + else + backend_mem_allocated -= deallocation; + + return; + } + + /* + * Do not allow backend_mem_allocated to go below zero. ereport if we + * would have. There's no need for a lock around the read here as it's + * being referenced from the same backend which means that there shouldn't + * be concurrent writes. We want to generate an ereport in these cases. + */ + if (deallocation > beentry->backend_mem_allocated) + { + ereport(LOG, errmsg("decrease reduces reported backend memory allocated below zero; setting reported to 0")); + + /* + * Overwrite deallocation with current backend_mem_allocated so we end + * up at zero. + */ + deallocation = beentry->backend_mem_allocated; + } + + /* + * Update my status entry, following the protocol of bumping + * st_changecount before and after. We use a volatile pointer here to + * ensure the compiler doesn't try to get cute. + */ + PGSTAT_BEGIN_WRITE_ACTIVITY(beentry); + beentry->backend_mem_allocated -= deallocation; + PGSTAT_END_WRITE_ACTIVITY(beentry); +} diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 4cca30aae7..1574aa8049 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -536,7 +536,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS) Datum pg_stat_get_activity(PG_FUNCTION_ARGS) { -#define PG_STAT_GET_ACTIVITY_COLS 30 +#define PG_STAT_GET_ACTIVITY_COLS 31 int num_backends = pgstat_fetch_stat_numbackends(); int curr_backend; int pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0); @@ -610,6 +610,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) else nulls[16] = true; + values[30] = UInt64GetDatum(beentry->backend_mem_allocated); + /* Values only available to role member or pg_read_all_stats */ if (HAS_PGSTAT_PERMISSIONS(beentry->st_userid)) { diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index b6eeb8abab..c91f8efa4d 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -47,6 +47,7 @@ #include "postgres.h" #include "port/pg_bitutils.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -509,6 +510,7 @@ AllocSetContextCreateInternal(MemoryContext parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; + pgstat_report_backend_mem_allocated_increase(firstBlockSize); return (MemoryContext) set; } @@ -532,6 +534,7 @@ AllocSetReset(MemoryContext context) AllocBlock block; Size keepersize PG_USED_FOR_ASSERTS_ONLY = set->keeper->endptr - ((char *) set); + uint64 deallocation = 0; AssertArg(AllocSetIsValid(set)); @@ -571,6 +574,7 @@ AllocSetReset(MemoryContext context) { /* Normal case, release the block */ context->mem_allocated -= block->endptr - ((char *) block); + deallocation += block->endptr - ((char *) block); #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -581,6 +585,7 @@ AllocSetReset(MemoryContext context) } Assert(context->mem_allocated == keepersize); + pgstat_report_backend_mem_allocated_decrease(deallocation); /* Reset block size allocation sequence, too */ set->nextBlockSize = set->initBlockSize; @@ -600,6 +605,7 @@ AllocSetDelete(MemoryContext context) AllocBlock block = set->blocks; Size keepersize PG_USED_FOR_ASSERTS_ONLY = set->keeper->endptr - ((char *) set); + uint64 deallocation = 0; AssertArg(AllocSetIsValid(set)); @@ -635,11 +641,13 @@ AllocSetDelete(MemoryContext context) freelist->first_free = (AllocSetContext *) oldset->header.nextchild; freelist->num_free--; + deallocation += oldset->header.mem_allocated; /* All that remains is to free the header/initial block */ free(oldset); } Assert(freelist->num_free == 0); + pgstat_report_backend_mem_allocated_decrease(deallocation); } /* Now add the just-deleted context to the freelist. */ @@ -656,7 +664,10 @@ AllocSetDelete(MemoryContext context) AllocBlock next = block->next; if (block != set->keeper) + { context->mem_allocated -= block->endptr - ((char *) block); + deallocation += block->endptr - ((char *) block); + } #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -669,6 +680,8 @@ AllocSetDelete(MemoryContext context) } Assert(context->mem_allocated == keepersize); + pgstat_report_backend_mem_allocated_decrease(deallocation + + context->mem_allocated); /* Finally, free the context header, including the keeper block */ free(set); @@ -712,6 +725,7 @@ AllocSetAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_backend_mem_allocated_increase(blksize); block->aset = set; block->freeptr = block->endptr = ((char *) block) + blksize; @@ -916,6 +930,7 @@ AllocSetAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_backend_mem_allocated_increase(blksize); block->aset = set; block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ; @@ -1016,6 +1031,7 @@ AllocSetFree(void *pointer) block->next->prev = block->prev; set->header.mem_allocated -= block->endptr - ((char *) block); + pgstat_report_backend_mem_allocated_decrease(block->endptr - ((char *) block)); #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -1127,7 +1143,9 @@ AllocSetRealloc(void *pointer, Size size) /* updated separately, not to underflow when (oldblksize > blksize) */ set->header.mem_allocated -= oldblksize; + pgstat_report_backend_mem_allocated_decrease(oldblksize); set->header.mem_allocated += blksize; + pgstat_report_backend_mem_allocated_increase(blksize); block->freeptr = block->endptr = ((char *) block) + blksize; diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c index b39894ec94..36e5b3f94d 100644 --- a/src/backend/utils/mmgr/generation.c +++ b/src/backend/utils/mmgr/generation.c @@ -37,6 +37,7 @@ #include "lib/ilist.h" #include "port/pg_bitutils.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -258,6 +259,7 @@ GenerationContextCreate(MemoryContext parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; + pgstat_report_backend_mem_allocated_increase(firstBlockSize); return (MemoryContext) set; } @@ -274,6 +276,7 @@ GenerationReset(MemoryContext context) { GenerationContext *set = (GenerationContext *) context; dlist_mutable_iter miter; + uint64 deallocation = 0; AssertArg(GenerationIsValid(set)); @@ -296,9 +299,14 @@ GenerationReset(MemoryContext context) if (block == set->keeper) GenerationBlockMarkEmpty(block); else + { + deallocation += block->blksize; GenerationBlockFree(set, block); + } } + pgstat_report_backend_mem_allocated_decrease(deallocation); + /* set it so new allocations to make use of the keeper block */ set->block = set->keeper; @@ -319,6 +327,9 @@ GenerationDelete(MemoryContext context) { /* Reset to release all releasable GenerationBlocks */ GenerationReset(context); + + pgstat_report_backend_mem_allocated_decrease(context->mem_allocated); + /* And free the context header and keeper block */ free(context); } @@ -355,6 +366,7 @@ GenerationAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_backend_mem_allocated_increase(blksize); /* block with a single (used) chunk */ block->context = set; @@ -458,6 +470,7 @@ GenerationAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_backend_mem_allocated_increase(blksize); /* initialize the new block */ GenerationBlockInit(set, block, blksize); @@ -691,6 +704,8 @@ GenerationFree(void *pointer) dlist_delete(&block->node); set->header.mem_allocated -= block->blksize; + pgstat_report_backend_mem_allocated_decrease(block->blksize); + free(block); } diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c index 2d70adef09..e0e69b394e 100644 --- a/src/backend/utils/mmgr/slab.c +++ b/src/backend/utils/mmgr/slab.c @@ -53,6 +53,7 @@ #include "postgres.h" #include "lib/ilist.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -218,6 +219,12 @@ SlabContextCreate(MemoryContext parent, parent, name); + /* + * If SlabContextCreate is updated to add headerSize to + * context->mem_allocated, then update here and SlabDelete appropriately + */ + pgstat_report_backend_mem_allocated_increase(headerSize); + return (MemoryContext) slab; } @@ -233,6 +240,7 @@ SlabReset(MemoryContext context) { int i; SlabContext *slab = castNode(SlabContext, context); + uint64 deallocation = 0; Assert(slab); @@ -258,9 +266,11 @@ SlabReset(MemoryContext context) free(block); slab->nblocks--; context->mem_allocated -= slab->blockSize; + deallocation += slab->blockSize; } } + pgstat_report_backend_mem_allocated_decrease(deallocation); slab->minFreeChunks = 0; Assert(slab->nblocks == 0); @@ -274,8 +284,17 @@ SlabReset(MemoryContext context) void SlabDelete(MemoryContext context) { + /* + * Until header allocation is included in context->mem_allocated, cast to + * slab and decrement the headerSize + */ + SlabContext *slab = castNode(SlabContext, context); + /* Reset to release all the SlabBlocks */ SlabReset(context); + + pgstat_report_backend_mem_allocated_decrease(slab->headerSize); + /* And free the context header */ free(context); } @@ -344,6 +363,7 @@ SlabAlloc(MemoryContext context, Size size) slab->minFreeChunks = slab->chunksPerBlock; slab->nblocks += 1; context->mem_allocated += slab->blockSize; + pgstat_report_backend_mem_allocated_increase(slab->blockSize); } /* grab the block from the freelist (even the new block is there) */ @@ -511,6 +531,7 @@ SlabFree(void *pointer) free(block); slab->nblocks--; slab->header.mem_allocated -= slab->blockSize; + pgstat_report_backend_mem_allocated_decrease(slab->blockSize); } else dlist_push_head(&slab->freelist[block->nfree], &block->node); diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index be47583122..e1bfb85b25 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5340,9 +5340,9 @@ proname => 'pg_stat_get_activity', prorows => '100', proisstrict => 'f', proretset => 't', provolatile => 's', proparallel => 'r', prorettype => 'record', proargtypes => 'int4', - proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8}', - proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id}', + proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8,int8}', + proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id,backend_mem_allocated}', prosrc => 'pg_stat_get_activity' }, { oid => '3318', descr => 'statistics: information about progress of backends running maintenance command', diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h index 7403bca25e..9bdc4197bd 100644 --- a/src/include/utils/backend_status.h +++ b/src/include/utils/backend_status.h @@ -168,6 +168,9 @@ typedef struct PgBackendStatus /* query identifier, optionally computed using post_parse_analyze_hook */ uint64 st_query_id; + + /* Current memory allocated to this backend */ + uint64 backend_mem_allocated; } PgBackendStatus; @@ -305,7 +308,9 @@ extern const char *pgstat_get_backend_current_activity(int pid, bool checkUser); extern const char *pgstat_get_crashed_backend_activity(int pid, char *buffer, int buflen); extern uint64 pgstat_get_my_query_id(void); - +extern void pgstat_report_backend_mem_allocated_increase(uint64 allocation); +extern void pgstat_report_backend_mem_allocated_decrease(uint64 deallocation); +extern uint64 pgstat_get_all_backend_memory_allocated(void); /* ---------- * Support functions for the SQL-callable functions to diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 7ec3d2688f..674e5c6fe7 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1757,9 +1757,10 @@ pg_stat_activity| SELECT s.datid, s.backend_xid, s.backend_xmin, s.query_id, + s.backend_mem_allocated, s.query, s.backend_type - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, backend_mem_allocated) LEFT JOIN pg_database d ON ((s.datid = d.oid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_all_indexes| SELECT c.oid AS relid, @@ -1871,7 +1872,7 @@ pg_stat_gssapi| SELECT s.pid, s.gss_auth AS gss_authenticated, s.gss_princ AS principal, s.gss_enc AS encrypted - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, backend_mem_allocated) WHERE (s.client_port IS NOT NULL); pg_stat_progress_analyze| SELECT s.pid, s.datid, @@ -2052,7 +2053,7 @@ pg_stat_replication| SELECT s.pid, w.sync_priority, w.sync_state, w.reply_time - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, backend_mem_allocated) JOIN pg_stat_get_wal_senders() w(pid, state, sent_lsn, write_lsn, flush_lsn, replay_lsn, write_lag, flush_lag, replay_lag, sync_priority, sync_state, reply_time) ON ((s.pid = w.pid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_replication_slots| SELECT s.slot_name, @@ -2086,7 +2087,7 @@ pg_stat_ssl| SELECT s.pid, s.ssl_client_dn AS client_dn, s.ssl_client_serial AS client_serial, s.ssl_issuer_dn AS issuer_dn - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, backend_mem_allocated) WHERE (s.client_port IS NOT NULL); pg_stat_subscription| SELECT su.oid AS subid, su.subname, -- 2.25.1
From 23d33838e3780ef02daad2bc737290d9905745a7 Mon Sep 17 00:00:00 2001 From: Reid Thompson <jreidthomp...@nc.rr.com> Date: Sat, 4 Jun 2022 22:23:59 -0400 Subject: [PATCH 2/2] Add the ability to limit the amount of memory that can be allocated to backends. This builds on the work that adds backend memory allocated to pg_stat_activity. Add GUC variable max_total_backend_memory. Specifies a limit to the amount of memory (in MB) that may be allocated to backends in total (i.e. this is not a per user or per backend limit). If unset, or set to 0 it is disabled. It is intended as a resource to help avoid the OOM killer on LINUX and manage resources in general. A backend request that would push the total over the limit will be denied with an out of memory error causing that backend's current query/transaction to fail. Due to the dynamic nature of memory allocations, this limit is not exact. If within 1.5MB of the limit and two backends request 1MB each at the same time both may be allocated, and exceed the limit. Further requests will not be allocated until dropping below the limit. Keep this in mind when setting this value. This limit does not affect auxiliary backend processes. Backend memory allocations are displayed in the pg_stat_activity view. --- doc/src/sgml/config.sgml | 26 +++++ src/backend/storage/ipc/dsm_impl.c | 12 ++ src/backend/utils/activity/backend_status.c | 109 +++++++++++++++++- src/backend/utils/misc/guc.c | 11 ++ src/backend/utils/misc/postgresql.conf.sample | 3 + src/backend/utils/mmgr/aset.c | 17 +++ src/backend/utils/mmgr/generation.c | 9 ++ src/backend/utils/mmgr/slab.c | 8 ++ src/include/utils/backend_status.h | 2 + 9 files changed, 196 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index a5cd4e44c7..e70ea71ba1 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2079,6 +2079,32 @@ include_dir 'conf.d' </listitem> </varlistentry> + <varlistentry id="guc-max-total-backend-memory" xreflabel="max_total_backend_memory"> + <term><varname>max_total_backend_memory</varname> (<type>integer</type>) + <indexterm> + <primary><varname>max_total_backend_memory</varname> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + Specifies a limit to the amount of memory (MB) that may be allocated to + backends in total (i.e. this is not a per user or per backend limit). + If unset, or set to 0 it is disabled. A backend request that would + push the total over the limit will be denied with an out of memory + error causing that backend's current query/transaction to fail. Due to + the dynamic nature of memory allocations, this limit is not exact. If + within 1.5MB of the limit and two backends request 1MB each at the same + time both may be allocated, and exceed the limit. Further requests will + not be allocated until dropping below the limit. Keep this in mind when + setting this value. This limit does not affect auxiliary backend + processes <xref linkend="glossary-auxiliary-proc"/> . Backend memory + allocations (<varname>backend_mem_allocated</varname>) are displayed in + the <link linkend="monitoring-pg-stat-activity-view"><structname>pg_stat_activity</structname></link> + view. + </para> + </listitem> + </varlistentry> + </variablelist> </sect2> diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c index 3356bb65b5..cc061056a3 100644 --- a/src/backend/storage/ipc/dsm_impl.c +++ b/src/backend/storage/ipc/dsm_impl.c @@ -253,6 +253,10 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size, return true; } + /* Do not exceed maximum allowed memory allocation */ + if (op == DSM_OP_CREATE && exceeds_max_total_bkend_mem(request_size)) + return false; + /* * Create new segment or open an existing one for attach. * @@ -524,6 +528,10 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, int flags = IPCProtection; size_t segsize; + /* Do not exceed maximum allowed memory allocation */ + if (op == DSM_OP_CREATE && exceeds_max_total_bkend_mem(request_size)) + return false; + /* * Allocate the memory BEFORE acquiring the resource, so that we don't * leak the resource if memory allocation fails. @@ -718,6 +726,10 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return true; } + /* Do not exceed maximum allowed memory allocation */ + if (op == DSM_OP_CREATE && exceeds_max_total_bkend_mem(request_size)) + return false; + /* Create new segment or open an existing one for attach. */ if (op == DSM_OP_CREATE) { diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index 45da3af213..9c2fc2f07c 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -44,6 +44,8 @@ */ bool pgstat_track_activities = false; int pgstat_track_activity_query_size = 1024; +/* Max backend memory allocation allowed (MB). 0 = disabled */ +int max_total_bkend_mem = 0; /* exposed so that backend_progress.c can access it */ @@ -1235,7 +1237,7 @@ pgstat_report_backend_mem_allocated_decrease(uint64 deallocation) */ if (deallocation > beentry->backend_mem_allocated) { - ereport(LOG, errmsg("decrease reduces reported backend memory allocated below zero; setting reported to 0")); + ereport(LOG, errmsg("deallocation would decrease backend memory below zero; setting reported to 0")); /* * Overwrite deallocation with current backend_mem_allocated so we end @@ -1253,3 +1255,108 @@ pgstat_report_backend_mem_allocated_decrease(uint64 deallocation) beentry->backend_mem_allocated -= deallocation; PGSTAT_END_WRITE_ACTIVITY(beentry); } + +/* ---------- + * pgstat_get_all_backend_memory_allocated() - + * + * Return a uint64 representing the current shared memory allocated to all + * backends. This looks directly at the BackendStatusArray, and so will + * provide current information regardless of the age of our transaction's + * snapshot of the status array. + * In the future we will likely utilize additional values - perhaps limit + * backend allocation by user/role, etc. + * ---------- + */ +uint64 +pgstat_get_all_backend_memory_allocated(void) +{ + PgBackendStatus *beentry; + int i; + uint64 all_backend_memory_allocated = 0; + + beentry = BackendStatusArray; + + /* + * We probably shouldn't get here before shared memory has been set up, + * but be safe. + */ + if (beentry == NULL || BackendActivityBuffer == NULL) + return 0; + + /* + * We include AUX procs in all backend memory calculation + */ + for (i = 1; i <= NumBackendStatSlots; i++) + { + /* + * We use a volatile pointer here to ensure the compiler doesn't try to + * get cute. + */ + volatile PgBackendStatus *vbeentry = beentry; + bool found; + uint64 backend_mem_allocated = 0; + + for (;;) + { + int before_changecount; + int after_changecount; + + pgstat_begin_read_activity(vbeentry, before_changecount); + + /* Ignore invalid entries, which may contain invalid data. + * See pgstat_beshutdown_hook() + */ + if (vbeentry->st_procpid > 0) + backend_mem_allocated = vbeentry->backend_mem_allocated; + + pgstat_end_read_activity(vbeentry, after_changecount); + + if ((found = pgstat_read_activity_complete(before_changecount, + after_changecount))) + break; + + /* Make sure we can break out of loop if stuck... */ + CHECK_FOR_INTERRUPTS(); + } + + if (found) + all_backend_memory_allocated += backend_mem_allocated; + + beentry++; + } + + return all_backend_memory_allocated; +} + +/* + * Determine if allocation request will exceed max backend memory allowed. + * Do not apply to auxiliary processes. + */ +bool +exceeds_max_total_bkend_mem(uint64 allocation_request) +{ + bool result = false; + + /* Exclude auxiliary processes from the check */ + if (MyAuxProcType != NotAnAuxProcess) + return result; + + /* Convert max_total_bkend_mem to bytes for comparison */ + if (max_total_bkend_mem && + pgstat_get_all_backend_memory_allocated() + + allocation_request > (uint64)max_total_bkend_mem * 1024 * 1024) + { + /* + * Explicitly identify the OOM being a result of this configuration + * parameter vs a system failure to allocate OOM. + */ + ereport(WARNING, + errmsg("allocation would exceed max_total_backend_memory limit (%llu > %llu)", + (unsigned long long)pgstat_get_all_backend_memory_allocated() + + allocation_request, (unsigned long long)max_total_bkend_mem * 1024 * 1024)); + + result = true; + } + + return result; +} diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 9fbbfb1be5..4e256a77d5 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -3664,6 +3664,17 @@ static struct config_int ConfigureNamesInt[] = NULL, NULL, NULL }, + { + {"max_total_backend_memory", PGC_SU_BACKEND, RESOURCES_MEM, + gettext_noop("Restrict total backend memory allocations to this max."), + gettext_noop("0 turns this feature off."), + GUC_UNIT_MB + }, + &max_total_bkend_mem, + 0, 0, INT_MAX, + NULL, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL, NULL diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 90bec0502c..8e944f6511 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -155,6 +155,9 @@ # mmap # (change requires restart) #min_dynamic_shared_memory = 0MB # (change requires restart) +#max_total_backend_memory = 0MB # Restrict total backend memory allocations + # to this max (in MB). 0 turns this feature + # off. # - Disk - diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index c91f8efa4d..ac9a1ced3f 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -428,6 +428,10 @@ AllocSetContextCreateInternal(MemoryContext parent, else firstBlockSize = Max(firstBlockSize, initBlockSize); + /* Do not exceed maximum allowed memory allocation */ + if (exceeds_max_total_bkend_mem(firstBlockSize)) + return NULL; + /* * Allocate the initial block. Unlike other aset.c blocks, it starts with * the context header and its block header follows that. @@ -720,6 +724,11 @@ AllocSetAlloc(MemoryContext context, Size size) { chunk_size = MAXALIGN(size); blksize = chunk_size + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ; + + /* Do not exceed maximum allowed memory allocation */ + if (exceeds_max_total_bkend_mem(blksize)) + return NULL; + block = (AllocBlock) malloc(blksize); if (block == NULL) return NULL; @@ -911,6 +920,10 @@ AllocSetAlloc(MemoryContext context, Size size) while (blksize < required_size) blksize <<= 1; + /* Do not exceed maximum allowed memory allocation */ + if (exceeds_max_total_bkend_mem(blksize)) + return NULL; + /* Try to allocate it */ block = (AllocBlock) malloc(blksize); @@ -1133,6 +1146,10 @@ AllocSetRealloc(void *pointer, Size size) blksize = chksize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ; oldblksize = block->endptr - ((char *) block); + /* Do not exceed maximum allowed memory allocation */ + if (blksize > oldblksize && exceeds_max_total_bkend_mem(blksize - oldblksize)) + return NULL; + block = (AllocBlock) realloc(block, blksize); if (block == NULL) { diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c index 36e5b3f94d..1d5720836c 100644 --- a/src/backend/utils/mmgr/generation.c +++ b/src/backend/utils/mmgr/generation.c @@ -192,6 +192,9 @@ GenerationContextCreate(MemoryContext parent, else allocSize = Max(allocSize, initBlockSize); + if (exceeds_max_total_bkend_mem(allocSize)) + return NULL; + /* * Allocate the initial block. Unlike other generation.c blocks, it * starts with the context header and its block header follows that. @@ -361,6 +364,9 @@ GenerationAlloc(MemoryContext context, Size size) { Size blksize = required_size + Generation_BLOCKHDRSZ; + if (exceeds_max_total_bkend_mem(blksize)) + return NULL; + block = (GenerationBlock *) malloc(blksize); if (block == NULL) return NULL; @@ -464,6 +470,9 @@ GenerationAlloc(MemoryContext context, Size size) if (blksize < required_size) blksize = pg_nextpower2_size_t(required_size); + if (exceeds_max_total_bkend_mem(blksize)) + return NULL; + block = (GenerationBlock *) malloc(blksize); if (block == NULL) diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c index e0e69b394e..63c07120dd 100644 --- a/src/backend/utils/mmgr/slab.c +++ b/src/backend/utils/mmgr/slab.c @@ -177,6 +177,10 @@ SlabContextCreate(MemoryContext parent, headerSize += chunksPerBlock * sizeof(bool); #endif + /* Do not exceed maximum allowed memory allocation */ + if (exceeds_max_total_bkend_mem(headerSize)) + return NULL; + slab = (SlabContext *) malloc(headerSize); if (slab == NULL) { @@ -331,6 +335,10 @@ SlabAlloc(MemoryContext context, Size size) */ if (slab->minFreeChunks == 0) { + /* Do not exceed maximum allowed memory allocation */ + if (exceeds_max_total_bkend_mem(slab->blockSize)) + return NULL; + block = (SlabBlock *) malloc(slab->blockSize); if (block == NULL) diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h index 9bdc4197bd..3b940ff98e 100644 --- a/src/include/utils/backend_status.h +++ b/src/include/utils/backend_status.h @@ -270,6 +270,7 @@ typedef struct LocalPgBackendStatus */ extern PGDLLIMPORT bool pgstat_track_activities; extern PGDLLIMPORT int pgstat_track_activity_query_size; +extern PGDLLIMPORT int max_total_bkend_mem; /* ---------- @@ -321,6 +322,7 @@ extern int pgstat_fetch_stat_numbackends(void); extern PgBackendStatus *pgstat_fetch_stat_beentry(int beid); extern LocalPgBackendStatus *pgstat_fetch_stat_local_beentry(int beid); extern char *pgstat_clip_activity(const char *raw_activity); +extern bool exceeds_max_total_bkend_mem(uint64 allocation_request); #endif /* BACKEND_STATUS_H */ -- 2.25.1