Remove Start* macros from postmaster.c to ease understanding of code
Hi,
Attached is a small patch implemented as I agree with Andres' comment
below noted while reviewing the thread
https://www.postgresql.org/message-id/flat/20240122210740.7vq5fd4woixpwx3f%40awork3.anarazel.de#6eb7595873392621d60e6b5a723941bc
I agree that its easier to not have to refer back to the macros only to
see that they're all invoking StartChildProcess(X). All invocations are
within postmaster.c.
> @@ -561,13 +561,13 @@ static void ShmemBackendArrayAdd(Backend *bn);
> static void ShmemBackendArrayRemove(Backend *bn);
> #endif /* EXEC_BACKEND
> */
>
> -#define StartupDataBase()StartChildProcess(StartupProcess)
> -#define StartArchiver()
> StartChildProcess(ArchiverProcess)
> -#define StartBackgroundWriter() StartChildProcess(BgWriterProcess)
> -#define StartCheckpointer() StartChildProcess(CheckpointerProcess)
> -#define StartWalWriter() StartChildProcess(WalWriterProcess)
> -#define StartWalReceiver() StartChildProcess(WalReceiverProcess)
> -#define StartWalSummarizer() StartChildProcess(WalSummarizerProcess)
> +#define StartupDataBase()StartChildProcess(B_STARTUP)
> +#define StartArchiver() StartChildProcess(B_ARCHIVER)
> +#define StartBackgroundWriter() StartChildProcess(B_BG_WRITER)
> +#define StartCheckpointer() StartChildProcess(B_CHECKPOINTER)
> +#define StartWalWriter() StartChildProcess(B_WAL_WRITER)
> +#define StartWalReceiver() StartChildProcess(B_WAL_RECEIVER)
> +#define StartWalSummarizer() StartChildProcess(B_WAL_SUMMARIZER)
Not for this commit, but we ought to rip out these macros - all they do is to
make it harder to understand the code.
From 236580a0dd381e245a459d0e8769bd30ba2d79e3 Mon Sep 17 00:00:00 2001
From: Reid Thompson
Date: Tue, 6 Feb 2024 09:17:28 -0500
Subject: [PATCH] Remove Start* macros in postmaster.c to ease understanding of
code. Per comment in thread here
https://www.postgresql.org/message-id/20240122210740.7vq5fd4woixpw...@awork3.anarazel.de
I agree that its easier to not have to refer back to the macros only to see
that they're just invoking StartChildProcess(X). All invocations are
within postmaster.c.
---
src/backend/postmaster/postmaster.c | 44 -
1 file changed, 18 insertions(+), 26 deletions(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index feb471dd1d..f1e60c7fd9 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -561,14 +561,6 @@ static void ShmemBackendArrayAdd(Backend *bn);
static void ShmemBackendArrayRemove(Backend *bn);
#endif /* EXEC_BACKEND */
-#define StartupDataBase() StartChildProcess(StartupProcess)
-#define StartArchiver() StartChildProcess(ArchiverProcess)
-#define StartBackgroundWriter() StartChildProcess(BgWriterProcess)
-#define StartCheckpointer() StartChildProcess(CheckpointerProcess)
-#define StartWalWriter() StartChildProcess(WalWriterProcess)
-#define StartWalReceiver() StartChildProcess(WalReceiverProcess)
-#define StartWalSummarizer() StartChildProcess(WalSummarizerProcess)
-
/* Macros to check exit status of a child process */
#define EXIT_STATUS_0(st) ((st) == 0)
#define EXIT_STATUS_1(st) (WIFEXITED(st) && WEXITSTATUS(st) == 1)
@@ -1457,14 +1449,14 @@ PostmasterMain(int argc, char *argv[])
/* Start bgwriter and checkpointer so they can help with recovery */
if (CheckpointerPID == 0)
- CheckpointerPID = StartCheckpointer();
+ CheckpointerPID = StartChildProcess(CheckpointerProcess);
if (BgWriterPID == 0)
- BgWriterPID = StartBackgroundWriter();
+ BgWriterPID = StartChildProcess(BgWriterProcess);
/*
* We're ready to rock and roll...
*/
- StartupPID = StartupDataBase();
+ StartupPID = StartChildProcess(StartupProcess);
Assert(StartupPID != 0);
StartupStatus = STARTUP_RUNNING;
pmState = PM_STARTUP;
@@ -1798,9 +1790,9 @@ ServerLoop(void)
pmState == PM_HOT_STANDBY || pmState == PM_STARTUP)
{
if (CheckpointerPID == 0)
-CheckpointerPID = StartCheckpointer();
+CheckpointerPID = StartChildProcess(CheckpointerProcess);
if (BgWriterPID == 0)
-BgWriterPID = StartBackgroundWriter();
+BgWriterPID = StartChildProcess(BgWriterProcess);
}
/*
@@ -1809,7 +1801,7 @@ ServerLoop(void)
* be writing any new WAL).
*/
if (WalWriterPID == 0 && pmState == PM_RUN)
- WalWriterPID = StartWalWriter();
+ WalWriterPID = StartChildProcess(WalWriterProcess);
/*
* If we have lost the autovacuum launcher, try to start a new one. We
@@ -1828,7 +1820,7 @@ ServerLoop(void)
/* If we have lost the archiver, try to start a new one. */
if (PgArchPID == 0 && PgArchStartupAllowed())
- PgArchPID = StartArchiver();
+ PgArchPID = StartChildProce
Re: Refactoring backend fork+exec code
On Thu, 2024-01-25 at 01:51 +0200, Heikki Linnakangas wrote: > > And here we go. BackendID is now a 1-based index directly into the > PGPROC array. > Would it be worthwhile to also note in this comment FIRST_AUX_PROC's and IsAuxProcess()'s dependency on B_ARCHIVER and it's location in the enum table? /* ¦* Auxiliary processes. These have PGPROC entries, but they are not ¦* attached to any particular database. There can be only one of each of ¦* these running at a time. ¦* ¦* If you modify these, make sure to update NUM_AUXILIARY_PROCS and the ¦* glossary in the docs. ¦*/ B_ARCHIVER, B_BG_WRITER, B_CHECKPOINTER, B_STARTUP, B_WAL_RECEIVER, B_WAL_SUMMARIZER, B_WAL_WRITER, } BackendType; #define BACKEND_NUM_TYPES (B_WAL_WRITER + 1) extern PGDLLIMPORT BackendType MyBackendType; #define FIRST_AUX_PROC B_ARCHIVER #define IsAuxProcess(type) (MyBackendType >= FIRST_AUX_PROC)
Re: Remove unused fields in ReorderBufferTupleBuf
On Fri, 2024-01-26 at 13:51 +0900, Masahiko Sawada wrote: > On Fri, Jan 26, 2024 at 1:24 PM wrote: > > > > On Thu, 2024-01-25 at 16:11 -0500, reid.thomp...@crunchydata.com wrote: > > > > > > I walked through v6 and didn't note any issues. > > Thank you for reviewing the patch! > Happy to. > > I'm not sure these changes are really beneficial. They contribute to > improving neither readability and performance IMO. > > Regards, > I thought that may be the case, but wanted to ask to be sure. Thank you for the followup. Reid
Re: Remove unused fields in ReorderBufferTupleBuf
On Thu, 2024-01-25 at 16:11 -0500, reid.thomp...@crunchydata.com wrote: > > I walked through v6 and didn't note any issues. > > I do want to ask, the patch alters ReorderBufferReturnTupleBuf() and > drops the unused parameter ReorderBuffer *rb. It seems that > ReorderBufferFreeSnap(), ReorderBufferReturnRelids(), > ReorderBufferImmediateInvalidation(), and ReorderBufferRestoreCleanup() > also pass ReorderBuffer *rb but do not use it. Would it be beneficial > to implement a separate patch to remove this parameter from these > functions also? > > Thanks, > Reid > It also appears that ReorderBufferSerializeChange() has 5 instances where it increments the local variables "data" but then they're never read. Lines 3806, 3836, 3854, 3889, 3910 I can create patch and post it to this thread or a new one if deemed worthwhile. Thanks, Reid
Re: Remove unused fields in ReorderBufferTupleBuf
On Thu, 2024-01-25 at 16:16 +0300, Aleksander Alekseev wrote:
> Hi,
>
> > > Here is the corrected patch.
> >
> > Thank you for updating the patch! I have some comments:
>
> Thanks for the review.
>
> > -tuple = (ReorderBufferTupleBuf *)
> > +tuple = (HeapTuple)
> > MemoryContextAlloc(rb->tup_context,
> > -
> > sizeof(ReorderBufferTupleBuf) +
> > + HEAPTUPLESIZE +
> > MAXIMUM_ALIGNOF +
> > alloc_len);
> > -tuple->alloc_tuple_size = alloc_len;
> > -tuple->tuple.t_data = ReorderBufferTupleBufData(tuple);
> > +tuple->t_data = (HeapTupleHeader)((char *)tuple + HEAPTUPLESIZE);
> >
> > Why do we need to add MAXIMUM_ALIGNOF? since HEAPTUPLESIZE is the
> > MAXALIGN'd size, it seems we don't need it. heap_form_tuple() does a
> > similar thing but it doesn't add it.
>
> Indeed. I gave it a try and nothing crashed, so it appears that
> MAXIMUM_ALIGNOF is not needed.
>
> > ---
> > xl_parameter_change *xlrec =
> > -(xl_parameter_change *)
> > XLogRecGetData(buf->record);
> > +(xl_parameter_change *)
> > XLogRecGetData(buf->record);
> >
> > Unnecessary change.
>
> That's pgindent. Fixed.
>
> > ---
> > -/*
> > - * Free a ReorderBufferTupleBuf.
> > - */
> > -void
> > -ReorderBufferReturnTupleBuf(ReorderBuffer *rb, ReorderBufferTupleBuf
> > *tuple)
> > -{
> > -pfree(tuple);
> > -}
> > -
> >
> > Why does ReorderBufferReturnTupleBuf need to be moved from
> > reorderbuffer.c to reorderbuffer.h? It seems not related to this
> > refactoring patch so I think we should do it in a separate patch if we
> > really want it. I'm not sure it's necessary, though.
>
> OK, fixed.
>
I walked through v6 and didn't note any issues.
I do want to ask, the patch alters ReorderBufferReturnTupleBuf() and
drops the unused parameter ReorderBuffer *rb. It seems that
ReorderBufferFreeSnap(), ReorderBufferReturnRelids(),
ReorderBufferImmediateInvalidation(), and ReorderBufferRestoreCleanup()
also pass ReorderBuffer *rb but do not use it. Would it be beneficial
to implement a separate patch to remove this parameter from these
functions also?
Thanks,
Reid
Re: [DOC] Add detail regarding resource consumption wrt max_connections
On Fri, 2024-01-19 at 17:37 -0500, reid.thomp...@crunchydata.com wrote: > On Sat, 2024-01-13 at 10:31 -0700, Roberto Mello wrote: > > > > I can add a suggestion for the user to consider increasing > > shared_buffers in accordance with higher max_connections, but it > > would be better if there was a "rule of thumb" guideline to go > > along. I'm open to suggestions. > > > > I can revise with a similar warning in max_prepared_xacts as well. > > > > Sincerely, > > > > Roberto > > Can a "close enough" rule of thumb be calculated from: > postgresql.conf -> log_min_messages = debug3 > > start postgresql with varying max_connections to get > CreateSharedMemoryAndSemaphores() sizes to generate a rough equation > or maybe it would be sufficient to advise to set log_min_messages = debug3 on a test DB and start/stop it with varying values of max_connections and look at the differing values in DEBUG: invoking IpcMemoryCreate(size=...) log messages for themselves.
Re: [DOC] Add detail regarding resource consumption wrt max_connections
On Sat, 2024-01-13 at 10:31 -0700, Roberto Mello wrote:
> On Fri, Jan 12, 2024 at 3:15 PM Cary Huang
> wrote:
> > I think it is good to warn the user about the increased allocation
> > of memory for certain parameters so that they do not abuse it by
> > setting it to a huge number without knowing the consequences.
> >
> > It is true that max_connections can increase the size of proc array
> > and other resources, which are allocated in the shared buffer,
> > which also means less shared buffer to perform regular data
> > operations. I am sure this is not the only parameter that affects
> > the memory allocation. "max_prepared_xacts" can also affect the
> > shared memory allocation too so the same warning message applies
> > here as well. Maybe there are other parameters with similar
> > effects.
> >
> > Instead of stating that higher max_connections results in higher
> > allocation, It may be better to tell the user that if the value
> > needs to be set much higher, consider increasing the
> > "shared_buffers" setting as well.
> >
>
>
> Appreciate the review, Cary.
>
> My goal was to inform the reader that there are implications to
> setting max_connections higher. I've personally seen a user
> mindlessly set this to 50k connections, unaware it would cause
> unintended consequences.
>
> I can add a suggestion for the user to consider increasing
> shared_buffers in accordance with higher max_connections, but it
> would be better if there was a "rule of thumb" guideline to go along.
> I'm open to suggestions.
>
> I can revise with a similar warning in max_prepared_xacts as well.
>
> Sincerely,
>
> Roberto
Can a "close enough" rule of thumb be calculated from:
postgresql.conf -> log_min_messages = debug3
start postgresql with varying max_connections to get
CreateSharedMemoryAndSemaphores() sizes to generate a rough equation
postgresql-12-main.log
max_connections=100
75:2024-01-19 17:04:56.544 EST [2762535] DEBUG: invoking
IpcMemoryCreate(size=149110784)
0.149110784GB
max_connections=1
1203:2024-01-19 17:06:13.502 EST [2764895] DEBUG: invoking
IpcMemoryCreate(size=644997120)
0.64499712GB
max_connections=2
5248:2024-01-19 17:24:27.956 EST [2954550] DEBUG: invoking
IpcMemoryCreate(size=1145774080)
1.14577408GB
max_connections=5
2331:2024-01-19 17:07:27.716 EST [2767079] DEBUG: invoking
IpcMemoryCreate(size=2591490048)
2.591490048GB
from lines 184-186
$ rg -B28 -A35 'invoking IpcMemoryCreate'
backend/storage/ipc/ipci.c
158-/*
159- * CreateSharedMemoryAndSemaphores
160- * Creates and initializes shared memory and semaphores.
161- *
162- * This is called by the postmaster or by a standalone backend.
163- * It is also called by a backend forked from the postmaster in the
164- * EXEC_BACKEND case. In the latter case, the shared memory segment
165- * already exists and has been physically attached to, but we have
to
166- * initialize pointers in local memory that reference the shared
structures,
167- * because we didn't inherit the correct pointer values from the
postmaster
168- * as we do in the fork() scenario. The easiest way to do that is
to run
169- * through the same code as before. (Note that the called routines
mostly
170- * check IsUnderPostmaster, rather than EXEC_BACKEND, to detect
this case.
171- * This is a bit code-wasteful and could be cleaned up.)
172- */
173-void
174-CreateSharedMemoryAndSemaphores(void)
175-{
176- PGShmemHeader *shim = NULL;
177-
178- if (!IsUnderPostmaster)
179- {
180- PGShmemHeader *seghdr;
181- Size size;
182- int numSemas;
183-
184- /* Compute the size of the shared-memory block */
185- size = CalculateShmemSize();
186: elog(DEBUG3, "invoking IpcMemoryCreate(size=%zu)", size);
187-
188- /*
189- * Create the shmem segment
190- */
191- seghdr = PGSharedMemoryCreate(size, );
192-
193- InitShmemAccess(seghdr);
194-
195- /*
196- * Create semaphores
197- */
198- PGReserveSemaphores(numSemas);
199-
200- /*
201- * If spinlocks are disabled, initialize emulation layer (which
202- * depends on semaphores, so the order is important here).
203- */
204-#ifndef HAVE_SPINLOCKS
205- SpinlockSemaInit();
206-#endif
207- }
208- else
209- {
210- /*
211- * We are reattaching to an existing shared memory segment. This
212- * should only be reached in the EXEC_BACKEND case.
213- */
214-#ifndef EXEC_BACKEND
215- elog(PANIC, "should be attached to shared memory already");
216-#endif
217- }
218-
219- /*
220- * Set up shared memory allocation mechanism
221- */
Re: DecodeInterval fixes
On Sun, 2023-07-09 at 13:24 -0400, Joseph Koshakow wrote: > > I've broken up this patch into three logical commits and attached > them. > None of the actual code has changed. > > Thanks, > Joe Koshakow I made a another pass through the separated patches, it looks good to me. Thanks, Reid
Re: DecodeInterval fixes
On Sat, 2023-07-08 at 13:18 -0400, Joseph Koshakow wrote: > Jacob Champion writes: > > Hi Joe, here's a partial review: > > Thanks so much for the review! > > > I'm new to this code, but I agree that the use of `type` and the > > lookahead are not particularly obvious/intuitive. At the very > > least, > > they'd need some more explanation in the code. Your boolean flag > > idea > > sounds reasonable, though. > > I've updated the patch with the boolean flag idea. I think it's a > bit cleaner and more readable. > > > > There is one more problem I noticed, but didn't fix. We allow > > > multiple > > > "@" to be sprinkled anywhere in the input, even though the docs > > > [0] > > > only allow it to appear at the beginning of the input. > > > > (No particular opinion on this.) > > I looked into this a bit. The reason this works is because the date > time lexer filters out all punctuation. That's what allows us to > parse > things like `SELECT date 'January 8, 1999';`. It's probably not worth > trying to be smarter about what punctuation we allow where, at least > for now. Maybe in the future we can exclude "@" from the punctuation > that get's filtered out. > > > It looks like this patch needs a rebase for the CI, too, but there > > are > > no conflicts. > > The attached patch is rebased against master. > > Thanks, > Joe Koshakow Apologies, I'm posting a little behind the curve here. My initial thoughts on the original patch mirrored Jacob's re 1 and 2 - that it looked good, did we need to consider the modified ecpg copy (which has been answered by Tom). I didn't have have any strong thought re 3) or the '@'. The updated patch looks good to me. Seems a little clearer/cleaner. Thanks, Reid
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Mon, 2023-05-22 at 08:42 -0400, reid.thomp...@crunchydata.com wrote: More followup to the above. > > I experimented on my system regarding > "The simple query select * from generate_series(0, 1000) shows roughly > 18.9 % degradation on my test server." > > My laptop: > 32GB ram > 11th Gen Intel(R) Core(TM) i7-11850H 8 cores/16 threads @ 2.50GHz (Max Turbo > Frequency. 4.80 GHz ; Cache. 24 MB) > SSD -> Model: KXG60ZNV1T02 NVMe KIOXIA 1024GB (nvme) Hi Ran through a few more tests on my system varying the initial_allocation_allowance and allocation_allowance_refill_qty from the current 1MB to 2, 4, 6, 8, 10 mb. Also realized that in my last tests/email I had posted percent difference rather than percent change. Turns out for the numbers that were being compared they're essentially the same, but I'm providing both for this set of tests. Ten runs for each comparison. Compared dev-max-memory set, dev-max-memory unset, master, and pg-stat-activity-backend-memory-allocated against master at each allocation value; Again, the test invokes psql -At -d postgres $connstr -P pager=off -c 'select * from generate_series(0, 1000)' 100 times on each of the 2 instances and calculates the AVG time and SD for the 100 runs. It then uses the AVG from each instance to calculate the percentage difference/change. These tests contain one code change not yet pushed to pgsql-hackers. In AllocSetReset() do not enter pgstat_report_allocated_bytes_decrease if no memory has been freed. Will format and post some pgbench test result in a separate email. Percent difference: ───┬─── │ Results: difference-dev-max-memory-set VS master ───┼─── 1 │ 1MB allocation2MB allocation4MB allocation 6MB allocation8MB allocation10MB allocation 2 │ 4.2263% difference 3.03961% difference 0.0585808% difference 2.92451% difference 3.34694% difference 2.67771% difference 3 │ 3.55709% difference 3.92339% difference 2.29144%difference 3.2156% difference 2.06153% difference 2.86217% difference 4 │ 2.04389% difference 2.91866% difference 3.73463%difference 2.86161% difference 3.60992% difference 3.07293% difference 5 │ 3.1306% difference 3.64773% difference 2.38063%difference 1.84845% difference 4.87375% difference 4.16953% difference 6 │ 3.12556% difference 3.34537% difference 2.99052%difference 2.60538% difference 2.14825% difference 1.95454% difference 7 │ 2.20615% difference 2.12861% difference 2.85282%difference 2.43336% difference 2.31389% difference 3.21563% difference 8 │ 1.9954% difference 3.61371% difference 3.35543%difference 3.49821% difference 3.41526% difference 8.25753% difference 9 │ 2.46845% difference 2.57784% difference 3.13067%difference 3.67681% difference 2.89139% difference 3.6067% difference 10 │ 3.60092% difference 2.16164% difference 3.9976% difference 2.6144% difference 4.27892% difference 2.68998% difference 11 │ 2.55454% difference 2.39073% difference 3.09631%difference 3.24292% difference 1.9107% difference 1.76182% difference 12 │ 13 │ 28.9089/1029.74729/10 27.888631/10 28.92125/10 30.85055/10 34.26854/10 14 │ 2.89089 2.974729 2.7888631 2.892125 3.085055 3.426854 ───┴─── ───┬─── │ Results: difference-dev-max-memory-unset VS master ───┼─── 1 │ 1MB allocation2MB allocation4MB allocation 6MB allocation8MB allocation10MB allocation 2 │ 3.96616% difference 3.05528% difference 0.563267% difference 1.12075% difference 3.52398% difference 3.25641% difference 3 │ 3.11387% difference 3.12499% difference 1.1133% difference 4.86997% difference 2.11481% difference 1.11668% difference 4 │ 3.14506% difference 2.06193% difference 3.36034%difference 2.80644% difference 2.37822% difference 3.07669% difference 5 │ 2.81052% difference 3.18499% difference 2.70705%difference 2.27847% difference 2.78506%
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Mon, 2023-05-22 at 08:42 -0400, reid.thomp...@crunchydata.com wrote: > On Wed, 2023-05-17 at 23:07 -0400, reid.thomp...@crunchydata.com wrote: > > Thanks for the feedback. > > > > I'm plannning to look at this. > > > > Is your benchmark something that I could utilize? I.E. is it a set of > > scripts or a standard test from somewhere that I can duplicate? > > > > Thanks, > > Reid > > Attach patches updated to master. Pulled from patch 2 back to patch 1 a change that was also pertinent to patch 1. From e6f8499e0270f2291494260bc341e8ad1411c2ae Mon Sep 17 00:00:00 2001 From: Reid Thompson Date: Thu, 11 Aug 2022 12:01:25 -0400 Subject: [PATCH 1/2] Add tracking of backend memory allocated Add tracking of backend memory allocated in total and by allocation type (aset, dsm, generation, slab) by process. allocated_bytes tracks the current bytes of memory allocated to the backend process. aset_allocated_bytes, dsm_allocated_bytes, generation_allocated_bytes and slab_allocated_bytes track the allocation by type for the backend process. They are updated for the process as memory is malloc'd/freed. Memory allocated to items on the freelist is included. 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. DSM allocations that are not destroyed by the creating process prior to it's exit are considered long lived and are tracked in a global counter global_dsm_allocated_bytes. We limit the floor of allocation counters to zero. Created views pg_stat_global_memory_allocation and pg_stat_memory_allocation for access to these trackers. --- doc/src/sgml/monitoring.sgml| 246 src/backend/catalog/system_views.sql| 34 +++ src/backend/storage/ipc/dsm.c | 11 +- src/backend/storage/ipc/dsm_impl.c | 78 +++ src/backend/storage/lmgr/proc.c | 1 + src/backend/utils/activity/backend_status.c | 114 + src/backend/utils/adt/pgstatfuncs.c | 84 +++ src/backend/utils/init/miscinit.c | 3 + src/backend/utils/mmgr/aset.c | 17 ++ src/backend/utils/mmgr/generation.c | 15 ++ src/backend/utils/mmgr/slab.c | 22 ++ src/include/catalog/pg_proc.dat | 17 ++ src/include/storage/proc.h | 2 + src/include/utils/backend_status.h | 144 +++- src/test/regress/expected/rules.out | 27 +++ src/test/regress/expected/stats.out | 36 +++ src/test/regress/sql/stats.sql | 20 ++ 17 files changed, 869 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index df5242fa80..cfc221fb2e 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -5757,6 +5757,252 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i + + pg_stat_memory_allocation + + + pg_stat_memory_allocation + + + + The pg_stat_memory_allocation view will have one + row per server process, showing information related to the current memory + allocation of that process in total and by allocator type. Due to the + dynamic nature of memory allocations the allocated bytes values may not be + exact but should be sufficient for the intended purposes. 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. Use + pg_size_pretty described in +to make these values more easily + readable. + + + + pg_stat_memory_allocation View + + + + + Column Type + + + Description + + + + + + + + datid oid + + + OID of the database this backend is connected to + + + + + + pid integer + + + Process ID of this backend + + + + + + allocated_bytes bigint + + + Memory currently allocated to this backend in bytes. This is the balance + of bytes allocated and freed by 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. + + + + + + aset_allocated_bytes bigint + + + Memory currently allocated to this backend in bytes via the allocation + set allocator. + + + + + + dsm_allocated_bytes bigint + + + Memory currently allocated to this backend in bytes via the dynamic + shared
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Wed, 2023-05-17 at 23:07 -0400, reid.thomp...@crunchydata.com wrote:
> Thanks for the feedback.
>
> I'm plannning to look at this.
>
> Is your benchmark something that I could utilize? I.E. is it a set of
> scripts or a standard test from somewhere that I can duplicate?
>
> Thanks,
> Reid
>
Hi Arne,
Followup to the above.
I experimented on my system regarding
"The simple query select * from generate_series(0, 1000) shows roughly 18.9
% degradation on my test server."
My laptop:
32GB ram
11th Gen Intel(R) Core(TM) i7-11850H 8 cores/16 threads @ 2.50GHz (Max Turbo
Frequency. 4.80 GHz ; Cache. 24 MB)
SSD -> Model: KXG60ZNV1T02 NVMe KIOXIA 1024GB (nvme)
I updated to latest master and rebased my patch branches.
I wrote a script to check out, build, install, init, and startup
master, patch 1, patch 1+2, patch 1+2 as master, pg-stats-memory,
dev-max-memory, and dev-max-memory-unset configured with
../../configure --silent
--prefix=/home/rthompso/src/git/postgres/install/${dir} --with-openssl
--with-tcl --with-tclconfig=/usr/lib/tcl8.6 --with-perl --with-libxml
--with-libxslt --with-python --with-gssapi --with-systemd --with-ldap
--enable-nls
where $dir in master, pg-stats-memory, and dev-max-memory,
dev-max-memory-unset.
The only change made to the default postgresql.conf was to have the
script add to the dev-max-memory instance the line
"max_total_backend_memory = 2048" before startup.
I did find one change in patch 2 that I pushed back into patch 1, this
should only impact the pg-stats-memory instance.
my .psqlrc turns timing on
I created a script where I can pass two instances to be compared.
It invokes
psql -At -d postgres $connstr -P pager=off -c 'select * from
generate_series(0, 1000)'
100 times on each of the 2 instances and calculates the AVG time and SD
for the 100 runs. It then uses the AVG from each instance to calculate
the percentage difference.
Depending on the instance, my results differ from master from
negligible to ~5.5%. Comparing master to itself had up to a ~2%
variation. See below.
12 runs comparing dev-max-memory 2048 VS master
Shows ~3% to 5.5% variation
Calculate average runtime percentage difference between VER dev-max-memory 2048
and VER master
1307.14 -> VER dev-max-memory 2048
1240.74 -> VER master
5.21218% difference
--
Calculate average runtime percentage difference between VER dev-max-memory 2048
and VER master
1315.99 -> VER dev-max-memory 2048
1245.64 -> VER master
5.4926% difference
--
Calculate average runtime percentage difference between VER dev-max-memory 2048
and VER master
1317.39 -> VER dev-max-memory 2048
1265.33 -> VER master
4.03141% difference
--
Calculate average runtime percentage difference between VER dev-max-memory 2048
and VER master
1313.52 -> VER dev-max-memory 2048
1256.69 -> VER master
4.42221% difference
--
Calculate average runtime percentage difference between VER dev-max-memory 2048
and VER master
1329.98 -> VER dev-max-memory 2048
1253.75 -> VER master
5.90077% difference
--
Calculate average runtime percentage difference between VER dev-max-memory 2048
and VER master
1314.47 -> VER dev-max-memory 2048
1245.6 -> VER master
5.38032% difference
--
Calculate average runtime percentage difference between VER dev-max-memory 2048
and VER master
1309.7 -> VER dev-max-memory 2048
1258.55 -> VER master
3.98326% difference
--
Calculate average runtime percentage difference between VER dev-max-memory 2048
and VER master
1322.16 -> VER dev-max-memory 2048
1248.94 -> VER master
5.69562% difference
--
Calculate average runtime percentage difference between VER dev-max-memory 2048
and VER master
1320.15 -> VER dev-max-memory 2048
1261.41 -> VER master
4.55074% difference
--
Calculate average runtime percentage difference between VER dev-max-memory 2048
and VER master
1345.22 -> VER dev-max-memory 2048
1280.96 -> VER master
4.8938% difference
--
Calculate average runtime percentage difference between VER dev-max-memory 2048
and VER master
1296.03 -> VER dev-max-memory 2048
1257.06 -> VER master
3.05277% difference
--
Calculate average runtime percentage difference between VER dev-max-memory 2048
and VER master
1319.5 -> VER dev-max-memory 2048
1252.34 -> VER master
5.22272% difference
12 showing dev-max-memory-unset VS master
Shows ~2.5% to 5% variation
Calculate average runtime percentage difference between VER dev-max-memory
unset and VER master
1300.93 -> VER dev-max-memory unset
1235.12 -> VER master
5.18996% difference
--
Calculate average runtime percentage difference between VER dev-max-memory
unset and VER master
1293.57 -> VER dev-max-memory unset
1263.93 -> VER master
2.31789% difference
--
Calculate average runtime percentage difference between VER dev-max-memory
unset and VER master
1303.05 -> VER dev-max-memory unset
1258.11 -> VER master
3.50935% difference
--
Calculate average runtime percentage difference between VER dev-max-memory
unset
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Wed, 2023-04-19 at 23:28 +, Arne Roland wrote: > > Thank you! I just tried our benchmark and got a performance > > degration > of around 28 %, which is way better than the last > > patch. > > > > The simple query select * from generate_series(0, 1000) shows > > > roughly 18.9 % degradation on my test server. > > > > By raising initial_allocation_allowance and > > > allocation_allowance_refill_qty I can get it to 16 % degradation. > > So > most of the degradation seems to be independent from raising > > the > allowance. > > > > I think we probably should investigate this further. > > > > Regards > > Arne > > Hi Arne, Thanks for the feedback. I'm plannning to look at this. Is your benchmark something that I could utilize? I.E. is it a set of scripts or a standard test from somewhere that I can duplicate? Thanks, Reid
Re: Add the ability to limit the amount of memory that can be allocated to backends.
Updated patches attached. Rebased to current master. Added additional columns to pg_stat_global_memory_allocation to summarize backend allocations by type. Updated documentation. Corrected some issues noted in review by John Morris. Added code re EXEC_BACKEND for dev-max-memory branch. From 34514ae2bebe5e3ab2a0b5b680d3932b5e7706ee Mon Sep 17 00:00:00 2001 From: Reid Thompson 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 tracking. 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 exhaust max_total_backend_memory memory will be denied with an out of memory error causing that backend's current query/transaction to fail. Further requests will not be allocated until dropping below the limit. Keep this in mind when setting this value. Due to the dynamic nature of memory allocations, this limit is not exact. This limit does not affect auxiliary backend processes. Backend memory allocations are displayed in the pg_stat_memory_allocation and pg_stat_global_memory_allocation views. --- doc/src/sgml/config.sgml | 30 +++ doc/src/sgml/monitoring.sgml | 38 +++- src/backend/catalog/system_views.sql | 2 + src/backend/port/sysv_shmem.c | 9 + src/backend/postmaster/postmaster.c | 5 + src/backend/storage/ipc/dsm_impl.c| 18 ++ src/backend/storage/lmgr/proc.c | 45 + src/backend/utils/activity/backend_status.c | 183 ++ src/backend/utils/adt/pgstatfuncs.c | 16 +- src/backend/utils/hash/dynahash.c | 3 +- src/backend/utils/init/miscinit.c | 8 + src/backend/utils/misc/guc_tables.c | 11 ++ src/backend/utils/misc/postgresql.conf.sample | 3 + src/backend/utils/mmgr/aset.c | 33 src/backend/utils/mmgr/generation.c | 16 ++ src/backend/utils/mmgr/slab.c | 15 +- src/include/catalog/pg_proc.dat | 6 +- src/include/storage/proc.h| 7 + src/include/utils/backend_status.h| 120 ++-- src/test/regress/expected/rules.out | 4 +- 20 files changed, 537 insertions(+), 35 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index bcc49aec45..4c735e180f 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2133,6 +2133,36 @@ include_dir 'conf.d' + + max_total_backend_memory (integer) + + max_total_backend_memory configuration parameter + + + + +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. At databse startup +max_total_backend_memory is reduced by shared_memory_size_mb +(includes shared buffers and other memory required for initialization). +Each backend process is intialized with a 1MB local allowance which +also reduces max_total_bkend_mem_bytes_available. Keep this in mind +when setting this value. A backend request that would exhaust the limit +will be denied with an out of memory error causing that backend's +current query/transaction to fail. Further requests will not be +allocated until dropping below the limit. This limit does not affect +auxiliary backend processes + or the postmaster process. +Backend memory allocations (allocated_bytes) are +displayed in the +pg_stat_memory_allocation +view. Due to the dynamic nature of memory allocations, this limit is +not exact. + + + + diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 70b3441412..704a75bd6e 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -5704,10 +5704,7 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i Memory currently allocated to this backend in bytes. This is the balance - of bytes allocated and freed by 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. + of bytes allocated and freed by this backend. @@ -5824,6 +5821,39 @@ SELECT pid, wait_event_type, wait_event
Re: FW: Add the ability to limit the amount of memory that can be allocated to backends.
On Thu, 2023-03-30 at 16:11 +, John Morris wrote:
> Hi Reid!
> Some thoughts
> I was looking at lmgr/proc.c, and I see a potential integer overflow
> - bothmax_total_bkend_mem and result are declared as “int”, so the
> expression “max_total_bkend_mem * 1024 * 1024 - result * 1024 * 1024”
> could have a problem whenmax_total_bkend_mem is set to 2G or more.
> /*
> * Account for shared
> memory size and initialize
> *
> max_total_bkend_mem_bytes.
> */
>
> pg_atomic_init_u64(>max_total_bkend_mem_bytes,
>
> max_total_bkend_mem *
> 1024 * 1024 - result * 1024 * 1024);
>
>
> As more of a style thing (and definitely not an error), the calling
> code might look smoother if the memory check and error handling were
> moved into a helper function, say “backend_malloc”. For example, the
> following calling code
>
> /* Do not exceed maximum allowed memory allocation */
> if
> (exceeds_max_total_bkend_mem(Slab_CONTEXT_HDRSZ(chunksPerBlock)))
> {
> MemoryContextStats(TopMemoryContext);
> ereport(ERROR,
>
> (errcode(ERRCODE_OUT_OF_MEMORY),
>
> errmsg("out of memory - exceeds max_total_backend_memory"),
>
> errdetail("Failed while creating memory context \"%s\".",
>
> name)));
> }
>
> slab = (SlabContext *)
> malloc(Slab_CONTEXT_HDRSZ(chunksPerBlock));
> if (slab == NULL)
> ….
> Could become a single line:
> Slab = (SlabContext *)
> backend_malloc(Slab_CONTEXT_HDRSZ(chunksPerBlock);
>
> Note this is a change in error handling as backend_malloc() throws
> memory errors. I think this change is already happening, as the error
> throws you’ve already added are potentially visible to callers (to be
> verified). It also could result in less informative error messages
> without the specifics of “while creating memory context”. Still, it
> pulls repeated code into a single wrapper and might be worth
> considering.
>
> I do appreciate the changes in updating the global memory counter. My
> recollection is the original version updated stats with every
> allocation, and there was something that looked like a spinlock
> around the update. (My memory may be wrong …). The new approach
> eliminates contention, both by having fewer updates and by using
> atomic ops. Excellent.
>
> I also have some thoughts about simplifying the atomic update logic
> you are currently using. I need to think about it a bit more and will
> get back to you later on that.
>
> * John Morris
>
>
>
>
John,
Thanks for looking this over and catching this. I appreciate the catch
and the guidance.
Thanks,
Reid
Re: Add the ability to limit the amount of memory that can be allocated to backends.
| 0 |0 5 | 752113 | active | | autovacuum worker| 5 | 752113 |27935608 | 27935608 | 0 | 0 |0 5 | 718693 | active | psql | client backend | 5 | 718693 | 8669976 | 8473240 | 196736 | 0 |0 (11 rows) From 4dd47f04764b5df9c3962d9fdb4096398bf85dfd Mon Sep 17 00:00:00 2001 From: Reid Thompson 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 tracking. 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 exhaust max_total_backend_memory memory will be denied with an out of memory error causing that backend's current query/transaction to fail. Further requests will not be allocated until dropping below the limit. Keep this in mind when setting this value. Due to the dynamic nature of memory allocations, this limit is not exact. This limit does not affect auxiliary backend processes. Backend memory allocations are displayed in the pg_stat_memory_allocation and pg_stat_global_memory_allocation views. --- doc/src/sgml/config.sgml | 28 +++ doc/src/sgml/monitoring.sgml | 48 - src/backend/catalog/system_views.sql | 6 +- src/backend/storage/ipc/dsm_impl.c| 18 ++ src/backend/storage/lmgr/proc.c | 45 + src/backend/utils/activity/backend_status.c | 173 ++ src/backend/utils/adt/pgstatfuncs.c | 16 +- src/backend/utils/hash/dynahash.c | 3 +- src/backend/utils/misc/guc_tables.c | 11 ++ src/backend/utils/misc/postgresql.conf.sample | 3 + src/backend/utils/mmgr/aset.c | 33 src/backend/utils/mmgr/generation.c | 16 ++ src/backend/utils/mmgr/slab.c | 16 +- src/include/catalog/pg_proc.dat | 6 +- src/include/storage/proc.h| 7 + src/include/utils/backend_status.h| 87 - src/test/regress/expected/rules.out | 4 +- 17 files changed, 499 insertions(+), 21 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 481f93cea1..9f37f6f070 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2113,6 +2113,34 @@ include_dir 'conf.d' + + max_total_backend_memory (integer) + + max_total_backend_memory configuration parameter + + + + +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. At databse startup +max_total_backend_memory is reduced by shared_memory_size_mb +(shared buffers). Each backend process is intialized with a 1MB local +allowance which also reduces max_total_bkend_mem_bytes_available. Keep +this in mind when setting this value. A backend request that would +exhaust the limit will be denied with an out of memory error causing +that backend's current query/transaction to fail. Further requests will +not be allocated until dropping below the limit. This limit does not +affect auxiliary backend processes +. Backend memory allocations +(allocated_bytes) are displayed in the +pg_stat_memory_allocation +view. Due to the dynamic nature of memory allocations, this limit is +not exact. + + + + diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index d943821071..a67bd484f2 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -5643,9 +5643,13 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i The pg_stat_memory_allocation view will have one row per server process, showing information related to the current memory - allocation of that process. Use pg_size_pretty - described in to make these values - more easily readable. + allocation of that process in total and by allocator type. 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. Use + pg_size_pretty described in +to make these values more
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Mon, 2023-02-13 at 16:26 -0800, Andres Freund wrote: > Hi, > > The tests recently started to fail: > > https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F42%2F3867 > > I marked this as waiting on author. > > Greetings, > > Andres Freund Patch has been rebased to master. The memory limiting portion (patch 0002-*) has been refactored to utilize a shared counter for total memory allocation along with backend-local allowances that are initialized at process startup and refilled from the central counter upon being used up. Free'd memory is accumulated and returned to the shared counter upon meeting a threshold and/or upon process exit. At this point arbitrarily picked 1MB as the initial allowance and return threshold. Thanks, Reid From e044bacedab503d1cd732146e1b9947406191bb6 Mon Sep 17 00:00:00 2001 From: Reid Thompson 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/postmaster/autovacuum.c | 8 +- src/backend/postmaster/postmaster.c | 17 +- src/backend/postmaster/syslogger.c| 4 +- src/backend/storage/ipc/dsm_impl.c| 35 ++- src/backend/storage/lmgr/proc.c | 3 + src/backend/utils/activity/backend_status.c | 223 +- src/backend/utils/misc/guc_tables.c | 11 + src/backend/utils/misc/postgresql.conf.sample | 3 + src/backend/utils/mmgr/aset.c | 43 +++- src/backend/utils/mmgr/generation.c | 21 +- src/backend/utils/mmgr/slab.c | 21 +- src/include/storage/proc.h| 7 + src/include/utils/backend_status.h| 222 +++-- 14 files changed, 560 insertions(+), 84 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index e5c41cc6c6..1bff68b1ec 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2113,6 +2113,32 @@ include_dir 'conf.d' + + max_total_backend_memory (integer) + + max_total_backend_memory configuration parameter + + + + +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 . Backend memory +allocations (allocated_bytes) are displayed in the +pg_stat_activity +view. + + + + diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 59c9bcf8c4..ee03d08dd9 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -407,8 +407,8 @@ StartAutoVacLauncher(void) #ifndef EXEC_BACKEND case 0: - /* Zero allocated bytes to avoid double counting parent allocation */ - pgstat_zero_my_allocated_bytes(); + /* Init allocated bytes to avoid double counting parent allocation */ + pgstat_init_allocated_bytes(); /* in postmaster child ... */ InitPostmasterChild(); @@ -1488,8 +1488,8 @@ StartAutoVacWorker(void) #ifndef EXEC_BACKEND case 0: - /* Zero allocated bytes to avoid double counting parent allocation */ -
Re: Add the ability to limit the amount of memory that can be allocated to backends.
Regarding the shared counter noted here, > What you could do is to have a single, imprecise, shared counter for the total > memory allocation, and have a backend-local "allowance". When the allowance is > used up, refill it from the shared counter (a single atomic op). Is there a preferred or suggested location to put variables like this? Perhaps a current variable to use as a reference? Thanks, Reid
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Mon, 2023-01-23 at 12:31 -0800, Andres Freund wrote: > Hi, > > I think it's basically still waiting on author, until the O(N) cost is gone > from the overflow limit check. > > Greetings, > > Andres Freund Yes, just a rebase. There is still work to be done per earlier in the thread. I do want to follow up and note re palloc/pfree vs malloc/free that the tracking code (0001-Add-tracking-...) is not tracking palloc/pfree but is explicitely tracking malloc/free. Not every palloc/pfree call executes the tracking code, only those where the path followed includes malloc() or free(). Routine palloc() calls fulfilled from the context's freelist/emptyblocks/freeblock/etc and pfree() calls not invoking free() avoid the tracking code. Thanks, Reid
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Thu, 2023-01-19 at 16:50 +0530, vignesh C wrote:
>
> The patch does not apply on top of HEAD as in [1], please post a rebased
> patch:
>
> Regards,
> Vignesh
rebased patch attached
Thanks,
Reid
From b32a346d6e0e00c568e9a285ad15fc2703998c26 Mon Sep 17 00:00:00 2001
From: Reid Thompson
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 | 108 ++
src/backend/utils/misc/guc_tables.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 | 9 +-
src/include/utils/backend_status.h| 3 +
9 files changed, 197 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f985afc009..51ed4623be 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -2113,6 +2113,32 @@ include_dir 'conf.d'
+
+ max_total_backend_memory (integer)
+
+ max_total_backend_memory configuration parameter
+
+
+
+
+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 . Backend memory
+allocations (allocated_bytes) are displayed in the
+pg_stat_activity
+view.
+
+
+
+
diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c
index 22885c7bd2..f7047107d5 100644
--- a/src/backend/storage/ipc/dsm_impl.c
+++ b/src/backend/storage/ipc/dsm_impl.c
@@ -254,6 +254,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.
*
@@ -525,6 +529,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.
@@ -719,6 +727,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 7baf2db57d..da2b5fb042 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -45,6 +45,9 @@
bool pgstat_track_activities = fals
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Mon, 2023-01-09 at 18:31 -0800, Andres Freund wrote:
> Hi,
>
> On 2023-01-05 13:44:20 -0500, Reid Thompson wrote:
> > 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.
>
> It doesn't actually malloc/free. It tracks palloc/pfree.
I will update the message
>
> > 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.
>
> As mentioned before, I don't think accounting DSM this way makes sense.
Understood, previously you noted 'There are a few uses of DSMs that track
shared resources, with the biggest likely being the stats for relations
etc'. I'd like to come up with a solution to address this; identifying the
long term allocations to shared state and accounting for them such that they
don't get 'lost' when the allocating backend exits. Any guidance or
direction would be appreciated.
> > --- a/src/backend/postmaster/autovacuum.c
> > +++ b/src/backend/postmaster/autovacuum.c
> > @@ -407,6 +407,9 @@ StartAutoVacLauncher(void)
> >
> > #ifndef EXEC_BACKEND
> > case 0:
> > + /* Zero allocated bytes to avoid double counting parent
> > allocation */
> > + pgstat_zero_my_allocated_bytes();
> > +
> > /* in postmaster child ... */
> > InitPostmasterChild();
>
>
>
> > @@ -1485,6 +1488,9 @@ StartAutoVacWorker(void)
> >
> > #ifndef EXEC_BACKEND
> > case 0:
> > + /* Zero allocated bytes to avoid double counting parent
> > allocation */
> > + pgstat_zero_my_allocated_bytes();
> > +
> > /* in postmaster child ... */
> > InitPostmasterChild();
> >
> > diff --git a/src/backend/postmaster/postmaster.c
> > b/src/backend/postmaster/postmaster.c
> > index eac3450774..24278e5c18 100644
> > --- a/src/backend/postmaster/postmaster.c
> > +++ b/src/backend/postmaster/postmaster.c
> > @@ -4102,6 +4102,9 @@ BackendStartup(Port *port)
> > {
> > free(bn);
> >
> > + /* Zero allocated bytes to avoid double counting parent
> > allocation */
> > + pgstat_zero_my_allocated_bytes();
> > +
> > /* Detangle from postmaster */
> > InitPostmasterChild();
>
>
> It doesn't at all seem right to call pgstat_zero_my_allocated_bytes() here,
> before even InitPostmasterChild() is called. Nor does it seem right to add the
> call to so many places.
>
> Note that this is before we even delete postmaster's memory, see e.g.:
> /*
>* If the PostmasterContext is still around, recycle the space; we don't
>* need it anymore after InitPostgres completes. Note this does not
> trash
>* *MyProcPort, because ConnCreate() allocated that space with malloc()
>* ... else we'd need to copy the Port data first. Also, subsidiary
> data
>* such as the username isn't lost either; see ProcessStartupPacket().
>*/
> if (PostmasterContext)
> {
> MemoryContextDelete(PostmasterContext);
> PostmasterContext = NULL;
> }
>
> calling pgstat_zero_my_allocated_bytes() before we do this will lead to
> undercounting memory usage, afaict.
>
OK - I'll trace back through these and see if I can better locate and reduce the
number of invocations.
> > +/* Enum helper for reporting memory allocated bytes */
> > +enum allocation_direction
> > +{
> > + PG_ALLOC_DECREASE = -1,
> > + PG_ALLOC_IGNORE,
> > + PG_ALLOC_INCREASE,
> > +};
>
> What's the point of this?
>
>
> > +/* --
> > + * pgstat_report_allocated_bytes() -
> > + *
> > + * Called to report change in memory allocated for this backend.
> > + *
> > + * my_allocated_bytes initially points to local memory, making it safe to
> > call
> > + * this before pgstats has been initialized. allocation_direction is a
> > + * positive/negative multiplier enum defined above.
> > + * --
> > + */
> > +static inline void
> > +pgstat_report_allocated_bytes(int64 allocated_bytes, int
> > allocation_direction)
>
> I don't think this should take allocation_direction as a parameter, I'd make
> it two different f
Re: Add tracking of backend memory allocated to pg_stat_activity
On Thu, 2023-01-05 at 14:13 -0600, Justin Pryzby wrote: > > I suggest to close the associated CF entry. Closed with status of Withdrawn. If that is invalid, please advise. > (Also, the people who participated in this thread may want to be > included in the other thread going forward.) Explicitly adding previously missed participants to Cc: - that conversation/patches are being consolidated to the thread "Add the ability to limit the amount of memory that can be allocated to backends" https://www.postgresql.org/message-id/bd57d9a4c219cc1392665fd5fba61dde8027b3da.ca...@crunchydata.com Thanks, Reid -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com
Re: Add tracking of backend memory allocated to pg_stat_activity
On Tue, 2023-01-03 at 16:25 +0530, vignesh C wrote: > ... > The patch does not apply on top of HEAD as in [1], please post a > rebased patch: >... > Regards, > Vignesh Per conversation in thread listed below, patches have been submitted to the "Add the ability to limit the amount of memory that can be allocated to backends" thread https://www.postgresql.org/message-id/bd57d9a4c219cc1392665fd5fba61dde8027b3da.ca...@crunchydata.com 0001-Add-tracking-of-backend-memory-allocated-to-pg_stat_.patch 0002-Add-the-ability-to-limit-the-amount-of-memory-that-c.patch On Thu, 8 Dec 2022 at 19:44, Reid Thompson wrote: > > On Sun, 2022-11-27 at 09:40 -0600, Justin Pryzby wrote: > > > ... > > > I still wonder whether there needs to be a separate CF entry for > > > the 0001 patch. One issue is that there's two different lists of > > > people involved in the threads. > > > > > I'm OK with containing the conversation to one thread if everyone else > is. If there's no argument against, then patches after today will go > to the "Add the ability to limit the amount of memory that can be > allocated to backends" thread > https://www.postgresql.org/message-id/bd57d9a4c219cc1392665fd5fba61dde8027b3da.ca...@crunchydata.com -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Tue, 2023-01-03 at 16:22 +0530, vignesh C wrote:
>
> The patch does not apply on top of HEAD as in [1], please post a
> rebased patch:
> ...
> Regards,
> Vignesh
>
Attached is rebased patch, with some updates related to committed changes.
Thanks,
Reid
--
Reid Thompson
Senior Software Engineer
Crunchy Data, Inc.
reid.thomp...@crunchydata.com
www.crunchydata.com
From 69516942b71d5d41850fbc00b971db7476c7a01a Mon Sep 17 00:00:00 2001
From: Reid Thompson
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 | 108 ++
src/backend/utils/misc/guc_tables.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 | 9 +-
src/include/utils/backend_status.h| 3 +
9 files changed, 197 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 05b3862d09..0362f26451 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -2079,6 +2079,32 @@ include_dir 'conf.d'
+
+ max_total_backend_memory (integer)
+
+ max_total_backend_memory configuration parameter
+
+
+
+
+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 . Backend memory
+allocations (allocated_bytes) are displayed in the
+pg_stat_activity
+view.
+
+
+
+
diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c
index 22885c7bd2..f7047107d5 100644
--- a/src/backend/storage/ipc/dsm_impl.c
+++ b/src/backend/storage/ipc/dsm_impl.c
@@ -254,6 +254,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.
*
@@ -525,6 +529,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.
@@ -719,6 +727,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 7baf2db5
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Tue, 2022-12-06 at 10:32 -0800, Andres Freund wrote: > Hi, > > On 2022-11-26 22:22:15 -0500, Reid Thompson wrote: > > rebased/patched to current master && current pg-stat-activity- > > backend-memory-allocated > > This version fails to build with msvc, and builds with warnings on > other > platforms. > https://cirrus-ci.com/build/5410696721072128 > msvc: > > Andres Freund updated patches -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com From e48292eaf402bfe397f1c2fdc1b3efd8cd0a9137 Mon Sep 17 00:00:00 2001 From: Reid Thompson 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 | 108 ++ src/backend/utils/misc/guc_tables.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| 3 + 9 files changed, 197 insertions(+) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index ff6fcd902a..2899f109ac 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2079,6 +2079,32 @@ include_dir 'conf.d' + + max_total_backend_memory (integer) + + max_total_backend_memory configuration parameter + + + + +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 . Backend memory +allocations (allocated_bytes) are displayed in the +pg_stat_activity +view. + + + + diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c index 36ef3e425e..58fb690c69 100644 --- a/src/backend/storage/ipc/dsm_impl.c +++ b/src/backend/storage/ipc/dsm_impl.c @@ -254,6 +254,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. * @@ -525,6 +529,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. @@ -719,6 +727,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 atta
Re: Add tracking of backend memory allocated to pg_stat_activity
On Sun, 2022-11-27 at 09:40 -0600, Justin Pryzby wrote: > > BTW, these should have some kind of prefix, like PG_ALLOC_* to > > avoid causing the same kind of problem for someone else that > > another header caused for you by defining something somewhere > > called IGNORE (ignore what, I don't know). The other problem was > > probably due to a define, though. Maybe instead of an enum, the > > function should take a boolean. > > Patch updated to current master and includes above prefix recommendation and combining of two function calls to one recommended by Ted Yu. > > > > I still wonder whether there needs to be a separate CF entry for > > the 0001 patch. One issue is that there's two different lists of > > people involved in the threads. > > I'm OK with containing the conversation to one thread if everyone else is. If there's no argument against, then patches after today will go to the "Add the ability to limit the amount of memory that can be allocated to backends" thread https://www.postgresql.org/message-id/bd57d9a4c219cc1392665fd5fba61dde8027b3da.ca...@crunchydata.com -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com From fdb7e6d5bb653e9c5031fd058bf168bdf80a20eb Mon Sep 17 00:00:00 2001 From: Reid Thompson Date: Thu, 11 Aug 2022 12:01:25 -0400 Subject: [PATCH] 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| 15 src/backend/catalog/system_views.sql| 1 + src/backend/postmaster/autovacuum.c | 6 ++ src/backend/postmaster/postmaster.c | 13 src/backend/postmaster/syslogger.c | 3 + src/backend/storage/ipc/dsm_impl.c | 81 + src/backend/utils/activity/backend_status.c | 45 src/backend/utils/adt/pgstatfuncs.c | 4 +- src/backend/utils/mmgr/aset.c | 17 + 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 | 59 ++- src/test/regress/expected/rules.out | 9 ++- src/test/regress/expected/stats.out | 11 +++ src/test/regress/sql/stats.sql | 3 + 16 files changed, 300 insertions(+), 9 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 11a8ebe5ec..13ecbe5877 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -948,6 +948,21 @@ postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser + + + allocated_bytes bigint + + + Memory currently allocated to this backend in bytes. This is the balance + of bytes allocated and freed by 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. Use pg_size_pretty + described in to make this value + more easily readable. + + + query text diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 2d8104b090..9ea8f78c95 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -864,6 +864,7 @@ CREATE VIEW pg_stat_activity AS S.state, S.backend_xid, s.backend_xmin, +S.allocated_bytes, S.query_id, S.query, S.backend_type diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 0746d80224..f6b6f71cdb 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -407,6 +407,9 @@ StartAutoVacLauncher(void) #ifndef EXEC_BACKEND case 0: + /* Zero allocated bytes to avoid double counting parent allocation */ + pgstat_zero_my_allocated_bytes(); + /* in postmaster child ... */ InitPostmasterChild(); @@ -1485,6 +1488,9 @@ StartAutoVacWorker(void) #ifndef EXEC_BACKEND
Re: Add tracking of backend memory allocated to pg_stat_activity
On Fri, 2022-12-02 at 09:18 -0800, Andres Freund wrote: > Hi, > > On 2022-12-02 11:09:30 -0500, Reid Thompson wrote: > > It appears to me that Postmaster populates the local variable that > > *my_allocated_bytes points to. That allocation is passed to forked > > children, and if not zeroed out, will be double counted as part of > > the child allocation. Is this invalid? > > I don't think we should count allocations made before > backend_status.c has > been initialized. > > Greetings, > > Andres Freund Hi, The intent was to capture and display all the memory allocated to the backends, for admins/users/max_total_backend_memory/others to utilize. Why should we ignore the allocations prior to backend_status.c? Thanks, Reid -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com
Re: Add tracking of backend memory allocated to pg_stat_activity
On Mon, 2022-11-28 at 10:59 -0800, Andres Freund wrote: > On 2022-11-26 22:10:06 -0500, Reid Thompson wrote: > > - zero allocated bytes after fork to avoid double counting > > postmaster allocations > > I still don't understand this - postmaster shouldn't be counted at > all. It > doesn't have a PgBackendStatus. There simply shouldn't be any tracked > allocations from it. > > Greetings, > > Andres Freund Hi Andres, I based this on the following. It appears to me that Postmaster populates the local variable that *my_allocated_bytes points to. That allocation is passed to forked children, and if not zeroed out, will be double counted as part of the child allocation. Is this invalid? $ ps -ef|grep postgres postgres6389 1 0 Dec01 ?00:00:17 /usr/sbin/pgbouncer -d /etc/pgbouncer/pgbouncer.ini rthompso 2937799 1 0 09:45 ?00:00:00 /tmp/postgres/install/pg-stats-memory/bin/postgres -D /var/tmp/pg-stats-memory -p 5433 rthompso 2937812 2937799 0 09:45 ?00:00:00 postgres: checkpointer rthompso 2937813 2937799 0 09:45 ?00:00:00 postgres: background writer rthompso 2937816 2937799 0 09:45 ?00:00:00 postgres: walwriter rthompso 2937817 2937799 0 09:45 ?00:00:00 postgres: autovacuum launcher rthompso 2937818 2937799 0 09:45 ?00:00:00 postgres: logical replication launcher rthompso 2938877 2636586 0 09:46 pts/400:00:00 /usr/lib/postgresql/12/bin/psql -h localhost -p 5433 postgres rthompso 2938909 2937799 0 09:46 ?00:00:00 postgres: rthompso postgres 127.0.0.1(44532) idle rthompso 2942164 1987403 0 09:49 pts/300:00:00 grep postgres Bracketing fork_process() calls with logging to print *my_allocated_bytes immediately prior and after fork_process... To me, this indicates that the forked children for this invocation (checkpointer, walwriter, autovac launcher, client backend, autovac worker, etc) are inheriting 240672 bytes from postmaster. $ ccat logfile 2022-12-02 09:45:05.871 EST [2937799] LOG: starting PostgreSQL 16devel on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0, 64-bit 2022-12-02 09:45:05.872 EST [2937799] LOG: listening on IPv4 address "127.0.0.1", port 5433 2022-12-02 09:45:05.874 EST [2937799] LOG: listening on Unix socket "/tmp/.s.PGSQL.5433" parent StartChildProcess process. pid: 2937799 *my_allocated_bytes: 240672 parent StartChildProcess process. pid: 2937799 *my_allocated_bytes: 240672 parent StartChildProcess process. pid: 2937799 *my_allocated_bytes: 240672 child StartChildProcess process. pid: 2937812 *my_allocated_bytes: 240672 child StartChildProcess process. pid: 2937813 *my_allocated_bytes: 240672 child StartChildProcess process. pid: 2937814 *my_allocated_bytes: 240672 2022-12-02 09:45:05.884 EST [2937814] LOG: database system was shut down at 2022-12-02 09:41:13 EST parent StartChildProcess process. pid: 2937799 *my_allocated_bytes: 240672 parent StartAutoVacLauncher process. pid: 2937799 *my_allocated_bytes: 240672 child StartChildProcess process. pid: 2937816 *my_allocated_bytes: 240672 parent do_start_bgworker process. pid: 2937799 *my_allocated_bytes: 240672 child StartAutoVacLauncher process. pid: 2937817 *my_allocated_bytes: 240672 2022-12-02 09:45:05.889 EST [2937799] LOG: database system is ready to accept connections child do_start_bgworker process. pid: 2937818 *my_allocated_bytes: 240672 parent StartAutoVacWorker process. pid: 2937799 *my_allocated_bytes: 240672 child StartAutoVacWorker process. pid: 2938417 *my_allocated_bytes: 240672 parent BackendStartup process. pid: 2937799 *my_allocated_bytes: 240672 child BackendStartup process. pid: 2938909 *my_allocated_bytes: 240672 parent StartAutoVacWorker process. pid: 2937799 *my_allocated_bytes: 240672 child StartAutoVacWorker process. pid: 2938910 *my_allocated_bytes: 240672 parent StartAutoVacWorker process. pid: 2937799 *my_allocated_bytes: 240672 child StartAutoVacWorker process. pid: 2939340 *my_allocated_bytes: 240672 parent StartAutoVacWorker process. pid: 2937799 *my_allocated_bytes: 240672 child StartAutoVacWorker process. pid: 2939665 *my_allocated_bytes: 240672 parent StartAutoVacWorker process. pid: 2937799 *my_allocated_bytes: 240672 child StartAutoVacWorker process. pid: 2940038 *my_allocated_bytes: 240672 parent StartAutoVacWorker process. pid: 2937799 *my_allocated_bytes: 240672 child StartAutoVacWorker process. pid: 2940364 *my_allocated_bytes: 240672 parent StartAutoVacWorker process. pid: 2937799 *my_allocated_bytes: 240672 child StartAutoVacWorker process. pid: 2940698 *my_allocated_bytes: 240672 parent StartAutoVacWorker process. pid: 2937799 *my_allocated_bytes: 240672 child StartAutoVacWorker process. pid: 2941317 *my_allocated_bytes: 240672 parent StartAutoVacWorker process. pid: 2937799 *my_allocated_bytes: 240672 child StartAutoVacWorker process. pid: 2941825 *my_allocated_bytes: 240
Re: Add tracking of backend memory allocated to pg_stat_activity
On Sat, 2022-11-26 at 22:10 -0500, Reid Thompson wrote: > Code rebased to current master. > Updated to incorporate additional recommendations from the the list > - add units to variables in view > - remove 'backend_' prefix from variables/functions > - update documentation > - add functional test for allocated_bytes > - refactor allocation reporting to reduce number of functions and > branches/reduce performance hit > - zero allocated bytes after fork to avoid double counting > postmaster allocations > > > > attempt to remedy cfbot windows build issues -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com From fb43761181925a87cb0674b6744b009fea614796 Mon Sep 17 00:00:00 2001 From: Reid Thompson Date: Thu, 11 Aug 2022 12:01:25 -0400 Subject: [PATCH] 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| 15 src/backend/catalog/system_views.sql| 1 + src/backend/postmaster/autovacuum.c | 6 ++ src/backend/postmaster/postmaster.c | 13 src/backend/postmaster/syslogger.c | 3 + src/backend/storage/ipc/dsm_impl.c | 81 + src/backend/utils/activity/backend_status.c | 45 src/backend/utils/adt/pgstatfuncs.c | 4 +- src/backend/utils/mmgr/aset.c | 17 + 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 | 58 ++- src/test/regress/expected/rules.out | 9 ++- src/test/regress/expected/stats.out | 11 +++ src/test/regress/sql/stats.sql | 3 + 16 files changed, 299 insertions(+), 9 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 5579b8b9e0..ffe7d2566c 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -947,6 +947,21 @@ postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser + + + allocated_bytes bigint + + + Memory currently allocated to this backend in bytes. This is the balance + of bytes allocated and freed by 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. Use pg_size_pretty + described in to make this value + more easily readable. + + + query text diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 2d8104b090..9ea8f78c95 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -864,6 +864,7 @@ CREATE VIEW pg_stat_activity AS S.state, S.backend_xid, s.backend_xmin, +S.allocated_bytes, S.query_id, S.query, S.backend_type diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 601834d4b4..f54606104d 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -407,6 +407,9 @@ StartAutoVacLauncher(void) #ifndef EXEC_BACKEND case 0: + /* Zero allocated bytes to avoid double counting parent allocation */ + pgstat_zero_my_allocated_bytes(); + /* in postmaster child ... */ InitPostmasterChild(); @@ -1485,6 +1488,9 @@ StartAutoVacWorker(void) #ifndef EXEC_BACKEND case 0: + /* Zero allocated bytes to avoid double counting parent allocation */ + pgstat_zero_my_allocated_bytes(); + /* in postmaster child ... */ InitPostmasterChild(); diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index a8a246921f..89a6caec78 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -4102,6 +4102,9 @@ BackendStartup(Port *por
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Thu, 2022-11-03 at 11:48 -0400, Reid Thompson wrote: > On Tue, 2022-10-25 at 11:49 -0400, Reid Thompson wrote: > > Rebased to current. Add a couple changes per conversation with D > Christensen (include units in field name, group field with > backend_xid > and backend_xmin fields in pg_stat_activity view, rather than between > query_id and query) > rebased/patched to current master && current pg-stat-activity-backend-memory-allocated -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com From 1470f45e086bef0757cc262d10e08904e46b9a88 Mon Sep 17 00:00:00 2001 From: Reid Thompson 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 | 108 ++ src/backend/utils/misc/guc_tables.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| 3 + 9 files changed, 197 insertions(+) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 24b1624bad..c2db3ace7a 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2079,6 +2079,32 @@ include_dir 'conf.d' + + max_total_backend_memory (integer) + + max_total_backend_memory configuration parameter + + + + +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 . Backend memory +allocations (allocated_bytes) are displayed in the +pg_stat_activity +view. + + + + diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c index 65d59fc43e..8d9df676af 100644 --- a/src/backend/storage/ipc/dsm_impl.c +++ b/src/backend/storage/ipc/dsm_impl.c @@ -254,6 +254,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. * @@ -525,6 +529,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. @@ -719,6 +727,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
Re: Add tracking of backend memory allocated to pg_stat_activity
On Wed, 2022-11-09 at 08:54 -0500, Reid Thompson wrote: > Hi Andres, > Thanks for looking at this and for the feedback. Responses inline > below. > >> On Fri, 2022-11-04 at 19:41 -0700, Andres Freund wrote: > > Hi, > > > On 2022-11-04 08:56:13 -0400, Reid Thompson wrote: > > > > I'm not convinced that counting DSM values this way is quite right. > > There are a few uses of DSMs that track shared resources, with the biggest > > likely being the stats for relations etc. I suspect that tracking that via > > backend memory usage will be quite confusing, because fairly random > > backends that > > had to grow the shared state end up being blamed with the memory usage in > > perpituity - and then suddenly that memory usage will vanish when that > > backend exits, > > despite the memory continuing to exist. > > Ok, I'll make an attempt to identify these allocations and manage > them elsewhere. still TBD > > > > > > > @@ -734,6 +747,7 @@ AllocSetAlloc(MemoryContext context, Size > > > size) > > > return NULL; > > > > > > context->mem_allocated += blksize; > > > + pgstat_report_backend_allocated_bytes_increase(bl > > > ksize); > > > > I suspect this will be noticable cost-wise. Even though these paths aren't > > the > > hottest memory allocation paths, by nature of going down into malloc, adding > > an external function call that then does a bunch of branching etc. seems > > likely to add up to some. See below for how I think we can deal with > > that... > > > > This is quite a few branches, including write/read barriers. > > > > It doesn't really make sense to use the > > PGSTAT_BEGIN_WRITE_ACTIVITY() pattern > > here - you're just updating a single value, there's nothing to be gained by > > it. The point of PGSTAT_BEGIN_*ACTIVITY() stuff is to allow readers to get a > > consistent view of multiple values - but there aren't multiple values > > here! > > I'll remove the barriers - initially I copied how prior functions were barriers removed > > > > To avoid the overhead of checking (!beentry || !pgstat_track_activities) and > > the external function call, I think you'd be best off copying the trickery I > > introduced for pgstat_report_wait_start(), in 225a22b19ed. > > > > I.e. make pgstat_report_backend_allocated_bytes_increase() a static inline > > function that unconditionally updates something like > > *my_backend_allocated_memory. To deal with the case of (!beentry || > > !pgstat_track_activities), that variable initially points to some backend > > local state and is set to the shared state in pgstat_bestart(). > > > > This additionally has the nice benefit that you can track memory usage from > > before pgstat_bestart(), it'll be in the local variable. > > OK, I think I can mimic the code you reference. done patch attached to https://www.postgresql.org/message-id/06b4922193b80776a31e08a3809f2414b0d4bf90.camel%40crunchydata.com -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com
Re: Add tracking of backend memory allocated to pg_stat_activity
On Wed, 2022-11-09 at 09:23 -0500, Reid Thompson wrote:
> Hi Melanie,
> Thank you for looking at this and for the feedback. Responses inline
> below.
>
> On Mon, 2022-11-07 at 16:17 -0500, Melanie Plageman wrote:
> >
> > It doesn't seem like you need the backend_ prefix in the view since
> > that is implied by it being in pg_stat_activity.
>
> I will remove the prefix.
done
>
> > For the wording on the description, I find "memory allocated to
> > this
> > backend" a bit confusing. Perhaps you could reword it to make clear
> > you mean that the number represents the balance of allocations by
> > this
> > backend. Something like:
> >
> > Memory currently allocated to this backend in bytes. This
> > is the
> > balance of bytes allocated and freed by this backend.
> > I would also link to the system administration function
> > pg_size_pretty() so users know how to easily convert the value.
>
> Thanks, I'll make these changes
done
> > > +/*
> > > + * pgstat_report_backend_allocated_bytes_increase() -
> > > + *
> > > + * Called to report increase in memory allocated for this
> > > backend
> > > + *
> > > + */
> >
> > It seems like you could combine the
> > pgstat_report_backend_allocated_bytes_decrease/increase() by either
> > using a signed integer to represent the allocation/deallocation or
> > passing in
> > a "direction" that is just a positive or negative multiplier enum.
> >
> > Especially if you don't use the write barriers, I think you could
> > simplify the logic in the two functions.
> >
> > If you do combine them, you might shorten the name to
> > pgstat_report_backend_allocation() or pgstat_report_allocation().
>
> Agreed. This seems a cleaner, simpler way to go. I'll add it to the
> TODO list.
done
>
> > > + /*
> > > + * Do not allow backend_allocated_bytes 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_allocated_bytes)
> > > + {
> > > + ereport(LOG, errmsg("decrease reduces reported
> > > backend memory allocated below zero; setting reported to 0"));
> > > +
> >
> > I also think it would be nice to include the deallocation amount
> > and
> > backend_allocated_bytes amount in the log message.
> > It also might be nice to start the message with something more
> > clear
> > than "decrease".
> > For example, I would find this clear as a user:
> >
> > Backend [backend_type or pid] deallocated [deallocation
> > number] bytes,
> > [backend_allocated_bytes - deallocation number] more than
> > this backend
> > has reported allocating.
>
> Sounds good, I'll implement these changes
done
> > > diff --git a/src/include/utils/backend_status.h
> > > b/src/include/utils/backend_status.h
> > > index b582b46e9f..75d87e8308 100644
> > > --- a/src/include/utils/backend_status.h
> > > +++ b/src/include/utils/backend_status.h
> > > @@ -169,6 +169,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_allocated_bytes;
> > > } PgBackendStatus;
> >
> > I don't think you need the backend_ prefix here since it is in
> > PgBackendStatus.
>
> Agreed again, I'll remove the prefix.
done
> > > /* --
> > > * 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 624d0e5aae..ba9f494806 100644
> > > --- a/src/test/regress/expected/rules.out
> > > +++ b/src/test/regress/expected/rules.out
> > > @@ -1753,10 +1753,11 @@ pg_stat_activity| SELECT s.datid,
> > > s.state,
> > > s.backend_xid,
> > > s.backend_xmin,
> > > + s.backend_allocated_bytes,
> > > s.query_id,
> > > s.query,
> > > s.backend_type
> >
> > Seems like it would be possible to add a functional test to
> > stats.sql.
>
> I will look at adding this.
done
patch attached to
https://www.postgresql.org/message-id/06b4922193b80776a31e08a3809f2414b0d4bf90.camel%40crunchydata.com
--
Reid Thompson
Senior Software Engineer
Crunchy Data, Inc.
reid.thomp...@crunchydata.com
www.crunchydata.com
Re: Add tracking of backend memory allocated to pg_stat_activity
Code rebased to current master.
Updated to incorporate additional recommendations from the the list
- add units to variables in view
- remove 'backend_' prefix from variables/functions
- update documentation
- add functional test for allocated_bytes
- refactor allocation reporting to reduce number of functions and
branches/reduce performance hit
- zero allocated bytes after fork to avoid double counting postmaster
allocations
--
Reid Thompson
Senior Software Engineer
Crunchy Data, Inc.
reid.thomp...@crunchydata.com
www.crunchydata.com
From 3d772d8620faba4bd4e091d6618c63557fbf6749 Mon Sep 17 00:00:00 2001
From: Reid Thompson
Date: Thu, 11 Aug 2022 12:01:25 -0400
Subject: [PATCH] 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| 15
src/backend/catalog/system_views.sql| 1 +
src/backend/postmaster/autovacuum.c | 6 ++
src/backend/postmaster/postmaster.c | 13
src/backend/postmaster/syslogger.c | 3 +
src/backend/storage/ipc/dsm_impl.c | 81 +
src/backend/utils/activity/backend_status.c | 45
src/backend/utils/adt/pgstatfuncs.c | 4 +-
src/backend/utils/mmgr/aset.c | 17 +
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 | 59 ++-
src/test/regress/expected/rules.out | 9 ++-
src/test/regress/expected/stats.out | 11 +++
src/test/regress/sql/stats.sql | 3 +
16 files changed, 300 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 5579b8b9e0..ffe7d2566c 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -947,6 +947,21 @@ postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser
+
+
+ allocated_bytes bigint
+
+
+ Memory currently allocated to this backend in bytes. This is the balance
+ of bytes allocated and freed by 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. Use pg_size_pretty
+ described in to make this value
+ more easily readable.
+
+
+
query text
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 2d8104b090..9ea8f78c95 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -864,6 +864,7 @@ CREATE VIEW pg_stat_activity AS
S.state,
S.backend_xid,
s.backend_xmin,
+S.allocated_bytes,
S.query_id,
S.query,
S.backend_type
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 601834d4b4..f54606104d 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -407,6 +407,9 @@ StartAutoVacLauncher(void)
#ifndef EXEC_BACKEND
case 0:
+ /* Zero allocated bytes to avoid double counting parent allocation */
+ pgstat_zero_my_allocated_bytes();
+
/* in postmaster child ... */
InitPostmasterChild();
@@ -1485,6 +1488,9 @@ StartAutoVacWorker(void)
#ifndef EXEC_BACKEND
case 0:
+ /* Zero allocated bytes to avoid double counting parent allocation */
+ pgstat_zero_my_allocated_bytes();
+
/* in postmaster child ... */
InitPostmasterChild();
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index a8a246921f..89a6caec78 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4102,6 +4102,9 @@ BackendStartup(Port *port)
{
free(bn);
+ /* Zero allocated bytes to avoid double counting parent allocation */
+ pgstat_zero_my_allocated_bytes();
+
/* Detangle from postmaster */
InitPostmasterChild();
@@ -5307,6 +5310,11 @@ StartChildProcess(AuxProcType type)
MemoryContextDelete
Re: Add tracking of backend memory allocated to pg_stat_activity
Hi Melanie,
Thank you for looking at this and for the feedback. Responses inline
below.
On Mon, 2022-11-07 at 16:17 -0500, Melanie Plageman wrote:
> On Fri, Nov 04, 2022 at 08:56:13AM -0400, Reid Thompson wrote:
> > From a8de5d29c0c6f10962181926a49ad4fec1e52bd1 Mon Sep 17 00:00:00
> > 2001
> > From: Reid Thompson
> > Date: Thu, 11 Aug 2022 12:01:25 -0400
> > Subject: [PATCH] Add tracking of backend memory allocated to
> > pg_stat_activity
> > +
>
> It doesn't seem like you need the backend_ prefix in the view since
> that is implied by it being in pg_stat_activity.
I will remove the prefix.
> For the wording on the description, I find "memory allocated to this
> backend" a bit confusing. Perhaps you could reword it to make clear
> you mean that the number represents the balance of allocations by this
> backend. Something like:
>
> Memory currently allocated to this backend in bytes. This is the
> balance of bytes allocated and freed by this backend.
> I would also link to the system administration function
> pg_size_pretty() so users know how to easily convert the value.
Thanks, I'll make these changes
> If you end up removing shared memory as Andres suggests in [1], I
> would link pg_shmem_allocations view here and point out that shared memory
> allocations can be viewed there instead (and why).
>
> You could instead add dynamic shared memory allocation to the
> pg_shmem_allocations view as suggested as follow-on work by the
> commit which introduced it, ed10f32e3.
>
> > +/*
> > + * pgstat_report_backend_allocated_bytes_increase() -
> > + *
> > + * Called to report increase in memory allocated for this backend
> > + *
> > + */
>
> It seems like you could combine the
> pgstat_report_backend_allocated_bytes_decrease/increase() by either
> using a signed integer to represent the allocation/deallocation or passing in
> a "direction" that is just a positive or negative multiplier enum.
>
> Especially if you don't use the write barriers, I think you could
> simplify the logic in the two functions.
>
> If you do combine them, you might shorten the name to
> pgstat_report_backend_allocation() or pgstat_report_allocation().
Agreed. This seems a cleaner, simpler way to go. I'll add it to the
TODO list.
> > + /*
> > + * Do not allow backend_allocated_bytes 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_allocated_bytes)
> > + {
> > + ereport(LOG, errmsg("decrease reduces reported
> > backend memory allocated below zero; setting reported to 0"));
> > +
>
> I also think it would be nice to include the deallocation amount and
> backend_allocated_bytes amount in the log message.
> It also might be nice to start the message with something more clear
> than "decrease".
> For example, I would find this clear as a user:
>
> Backend [backend_type or pid] deallocated [deallocation number] bytes,
> [backend_allocated_bytes - deallocation number] more than this backend
> has reported allocating.
Sounds good, I'll implement these changes
> > diff --git a/src/backend/utils/adt/pgstatfuncs.c
> > b/src/backend/utils/adt/pgstatfuncs.c
> > index 96bffc0f2a..b6d135ad2f 100644
> > --- a/src/backend/utils/adt/pgstatfuncs.c
> > +++ b/src/backend/utils/adt/pgstatfuncs.c
> > @@ -553,7 +553,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 =
> >
> > + values[30] =
> > UInt64GetDatum(beentry->backend_allocated_bytes);
>
> Though not the fault of this patch, it is becoming very difficult to
> keep the columns straight in pg_stat_get_activity(). Perhaps you
> could add a separate commit to add an enum for the columns so the function
> is easier to understand.
>
> > diff --git a/src/include/utils/backend_status.h
> > b/src/include/utils/backend_status.h
> > index b582b46e9f..75d87e8308 100644
> > --- a/src/include/utils/backend_status.h
> > +++ b/src/include/utils/backend_status.h
> > @@ -169,6 +
Re: Add tracking of backend memory allocated to pg_stat_activity
Hi Andres,
Thanks for looking at this and for the feedback. Responses inline below.
On Fri, 2022-11-04 at 19:41 -0700, Andres Freund wrote:
> Hi,
>
> On 2022-11-04 08:56:13 -0400, Reid Thompson wrote:
>
> I'm not convinced that counting DSM values this way is quite right.
> There are a few uses of DSMs that track shared resources, with the biggest
> likely being the stats for relations etc. I suspect that tracking that via
> backend memory usage will be quite confusing, because fairly random backends
> that
> had to grow the shared state end up being blamed with the memory usage in
> perpituity - and then suddenly that memory usage will vanish when that
> backend exits,
> despite the memory continuing to exist.
Ok, I'll make an attempt to identify these allocations and manage them
elsewhere.
>
>
> > @@ -734,6 +747,7 @@ AllocSetAlloc(MemoryContext context, Size size)
> > return NULL;
> >
> > context->mem_allocated += blksize;
> > + pgstat_report_backend_allocated_bytes_increase(blksize);
> >
> > block->aset = set;
> > block->freeptr = block->endptr = ((char *) block) + blksize;
> > @@ -944,6 +958,7 @@ AllocSetAlloc(MemoryContext context, Size size)
> > return NULL;
> >
> > context->mem_allocated += blksize;
> > + pgstat_report_backend_allocated_bytes_increase(blksize);
> >
> > block->aset = set;
> > block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ;
> > @@ -1043,6 +1058,7 @@ AllocSetFree(void *pointer)
> > block->next->prev = block->prev;
> >
> > set->header.mem_allocated -= block->endptr - ((char *)
> > block);
> > +
> > pgstat_report_backend_allocated_bytes_decrease(block->endptr - ((char *)
> > block));
> >
> > #ifdef CLOBBER_FREED_MEMORY
> > wipe_mem(block, block->freeptr - ((char *) block));
>
> I suspect this will be noticable cost-wise. Even though these paths aren't the
> hottest memory allocation paths, by nature of going down into malloc, adding
> an external function call that then does a bunch of branching etc. seems
> likely to add up to some. See below for how I think we can deal with
> that...
>
>
> > +
> > +/*
> > + * pgstat_report_backend_allocated_bytes_increase() -
> > + *
> > + * Called to report increase in memory allocated for this backend
> > + *
> > + */
> > +void
> > +pgstat_report_backend_allocated_bytes_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_allocated_bytes += 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_allocated_bytes += allocation;
> > + PGSTAT_END_WRITE_ACTIVITY(beentry);
> > +}
>
> This is quite a few branches, including write/read barriers.
>
> It doesn't really make sense to use the PGSTAT_BEGIN_WRITE_ACTIVITY() pattern
> here - you're just updating a single value, there's nothing to be gained by
> it. The point of PGSTAT_BEGIN_*ACTIVITY() stuff is to allow readers to get a
> consistent view of multiple values - but there aren't multiple values
> here!
I'll remove the barriers - initially I copied how prior functions were
coded as my template ala
pgstat_report_query_id, pgstat_report_xact_timestamp.
>
> To avoid the overhead of checking (!beentry || !pgstat_track_activities) and
> the external function call, I think you'd be best off copying the trickery I
> introduced for pgstat_report_wait_start(), in 225a22b19ed.
>
> I.e. make pgstat_report_backend_allocated_bytes_increase() a static inline
> function that unconditionally updates something like
> *my_backend_allocated_memory. To deal with the case of (!beentry ||
> !pgstat_track_activities), that
Re: Add tracking of backend memory allocated to pg_stat_activity
On Fri, 2022-11-04 at 11:06 +0100, Drouvot, Bertrand wrote:
> Hi,
>
> It looks like the patch does not apply anymore since b1099eca8f.
>
> Regards,
>
Thanks,
rebased to current master attached.
--
Reid Thompson
Senior Software Engineer
Crunchy Data, Inc.
reid.thomp...@crunchydata.com
www.crunchydata.com
From a8de5d29c0c6f10962181926a49ad4fec1e52bd1 Mon Sep 17 00:00:00 2001
From: Reid Thompson
Date: Thu, 11 Aug 2022 12:01:25 -0400
Subject: [PATCH] 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 | 81 +++
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, 270 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index e5d622d514..972805b85a 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
+
+
+ backend_allocated_bytes bigint
+
+
+ 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.
+
+
+
query text
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 2d8104b090..84d462aa97 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -864,6 +864,7 @@ CREATE VIEW pg_stat_activity AS
S.state,
S.backend_xid,
s.backend_xmin,
+S.backend_allocated_bytes,
S.query_id,
S.query,
S.backend_type
diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c
index 6ddd46a4e7..eae03159c3 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,14 @@ 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_allocated_bytes_decrease(*mapped_size);
*mapped_address = NULL;
*mapped_size = 0;
if (op == DSM_OP_DESTROY && shm_unlink(name) != 0)
@@ -332,6 +341,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_allocated_bytes_decr
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Tue, 2022-10-25 at 11:49 -0400, Reid Thompson wrote: > Hi Arne, > > On Mon, 2022-10-24 at 15:27 +, Arne Roland wrote: > > Hello Reid, > > > > could you rebase the patch again? It doesn't apply currently > > (http://cfbot.cputube.org/patch_40_3867.log). Thanks! > > rebased patches attached. Rebased to current. Add a couple changes per conversation with D Christensen (include units in field name, group field with backend_xid and backend_xmin fields in pg_stat_activity view, rather than between query_id and query) -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com From 9cf35c79be107feedb63f6f674ac9d2347d1875e Mon Sep 17 00:00:00 2001 From: Reid Thompson 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 | 111 +- src/backend/utils/misc/guc_tables.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, 198 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 559eb898a9..5762999fa5 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2079,6 +2079,32 @@ include_dir 'conf.d' + + max_total_backend_memory (integer) + + max_total_backend_memory configuration parameter + + + + +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 . Backend memory +allocations (backend_allocated_bytes) are displayed in +the pg_stat_activity +view. + + + + diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c index eae03159c3..aaf74e9486 100644 --- a/src/backend/storage/ipc/dsm_impl.c +++ b/src/backend/storage/ipc/dsm_impl.c @@ -254,6 +254,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. * @@ -525,6 +529,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. @@ -719,6 +727,10 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return true; } + /* Do not exceed maximum allowed memory allocation */ + if (op == DS
Re: Add tracking of backend memory allocated to pg_stat_activity
On Tue, 2022-10-25 at 14:51 -0400, Reid Thompson wrote:
> patch rebased to current master
>
actually attach the patch
--
Reid Thompson
Senior Software Engineer
Crunchy Data, Inc.
reid.thomp...@crunchydata.com
www.crunchydata.com
From ab654a48ec7bfbc3bc377c5757a04f1756e72e79 Mon Sep 17 00:00:00 2001
From: Reid Thompson
Date: Thu, 11 Aug 2022 12:01:25 -0400
Subject: [PATCH] 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 e5d622d514..4983bbc814 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
+
+
+ backend_mem_allocated bigint
+
+
+ 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.
+
+
+
query text
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 2d8104b090..cbf804625c 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -865,6 +865,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(re
Re: Add tracking of backend memory allocated to pg_stat_activity
patch rebased to current master -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com
Re: Add the ability to limit the amount of memory that can be allocated to backends.
Hi Arne,
On Mon, 2022-10-24 at 15:27 +, Arne Roland wrote:
> Hello Reid,
>
> could you rebase the patch again? It doesn't apply currently
> (http://cfbot.cputube.org/patch_40_3867.log). Thanks!
rebased patches attached.
> You mention, that you want to prevent the compiler from getting
> cute.I don't think this comments are exactly helpful in the current
> state. I think probably fine to just omit them.
I attempted to follow previous convention when adding code and these
comments have been consistently applied throughout backend_status.c
where a volatile pointer is being used.
> I don't understand the purpose of the result variable in
> exceeds_max_total_bkend_mem. What purpose does it serve?
>
> I really like the simplicity of the suggestion here to prevent oom.
If max_total_backend_memory is configured, exceeds_max_total_bkend_mem()
will return true if an allocation request will push total backend memory
allocated over the configured value.
exceeds_max_total_bkend_mem() is implemented in the various allocators
along the lines of
...snip...
/* Do not exceed maximum allowed memory allocation */
if (exceeds_max_total_bkend_mem('new request size'))
return NULL;
...snip...
Do not allocate the memory requested, return NULL instead. PG already
had code in place to handle NULL returns from allocation requests.
The allocation code in aset.c, slab.c, generation.c, dsm_impl.c utilizes
exceeds_max_total_bkend_mem()
max_total_backend_memory (integer)
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 Auxiliary process . Backend memory allocations
(backend_mem_allocated) are displayed in the pg_stat_activity view.
> I intent to play around with a lot of backends, once I get a rebased
> patch.
>
> Regards
> Arne
From ab654a48ec7bfbc3bc377c5757a04f1756e72e79 Mon Sep 17 00:00:00 2001
From: Reid Thompson
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 e5d622d514..4983bbc814 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
+
+
+ backend_mem_allocated bigint
+
+
+ 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.
+
+
+
query text
diff --git a/src/backen
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Thu, 2022-09-15 at 12:07 +0400, Ibrar Ahmed wrote:
>
> The patch does not apply; please rebase the patch.
>
> patching file src/backend/utils/misc/guc.c
> Hunk #1 FAILED at 3664.
> 1 out of 1 hunk FAILED -- saving rejects to file
> src/backend/utils/misc/guc.c.rej
>
> patching file src/backend/utils/misc/postgresql.conf.sample
>
rebased patches attached.
Thanks,
Reid
From 0e7010c53508d5a396edd16fd9166abe431f5dbe Mon Sep 17 00:00:00 2001
From: Reid Thompson
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
+
+
+ backend_mem_allocated bigint
+
+
+ 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.
+
+
+
query text
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 55f7ec79e0..a78750ab12 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -862,6 +862,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.
+
Re: Add the ability to limit the amount of memory that can be allocated to backends.
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
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
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Fri, 2022-09-02 at 09:30 +0200, Drouvot, Bertrand wrote: > Hi, > > I'm not sure we are choosing the right victims here (aka the ones > that are doing the request that will push the total over the limit). > > Imagine an extreme case where a single backend consumes say 99% of > the limit, shouldn't it be the one to be "punished"? (and somehow forced > to give the memory back). > > The problem that i see with the current approach is that a "bad" > backend could impact all the others and continue to do so. > > what about punishing say the highest consumer , what do you think? > (just speaking about the general idea here, not about the implementation) Initially, we believe that punishing the detector is reasonable if we can help administrators avoid the OOM killer/resource starvation. But we can and should expand on this idea. Another thought is, rather than just failing the query/transaction we have the affected backend do a clean exit, freeing all it's resources. -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com
Re: Add the ability to limit the amount of memory that can be allocated to backends.
On Thu, 2022-09-01 at 11:48 +0900, Kyotaro Horiguchi wrote: > > > > The patch seems to limit both of memory-context allocations and DSM > > allocations happen on a specific process by the same budget. In the > > fist place I don't think it's sensible to cap the amount of DSM > > allocations by per-process budget. > > > > DSM is used by pgstats subsystem. There can be cases where pgstat > > complains for denial of DSM allocation after the budget has been > > exhausted by memory-context allocations, or every command complains > > for denial of memory-context allocation after once the per-process > > budget is exhausted by DSM allocations. That doesn't seem > > reasonable. > > regards. It's intended as a mechanism for administrators to limit total postgresql memory consumption to avoid the OOM killer causing a crash and restart, or to ensure that resources are available for other processes on shared hosts, etc. It limits all types of allocations in order to accomplish this. Our documentation will note this, so that administrators that have the need to set it are aware that it can affect all non-auxiliary processes and what the effect is.
Re: Add tracking of backend memory allocated to pg_stat_activity
On Thu, 2022-09-01 at 13:43 +0900, Kyotaro Horiguchi wrote: > > > @@ -916,6 +930,7 @@ AllocSetAlloc(MemoryContext context, Size size) > > return NULL; > > > > context->mem_allocated += blksize; > > + pgstat_report_backend_mem_allocated_increase(blksi > > ze); > > I'm not sure this is acceptable. The function adds a branch even when > the feature is turned off, which I think may cause a certain extent > of > performance degradation. A past threads [1], [2] and [3] might be > informative. Stated above is '...even when the feature is turned off...', I want to note that this feature/patch (for tracking memory allocated) doesn't have an 'on/off'. Tracking would always occur. I'm open to guidance on testing for performance degradation. I did note some basic pgbench comparison numbers in the thread regarding limiting backend memory allocations. > -- > Kyotaro Horiguchi > NTT Open Source Software Center
Re: Add the ability to limit the amount of memory that can be allocated to backends.
om Master | Max Mem off | Diff from Master | Max Mem 100MB | Diff from Master | | Set 1| Test 1 | Test 2 | | Test 3 | | Test 4| | | latency average | 2.43390909090909 | 2.44327272727273 | | 2.44381818181818 | | 2.6843| | | tps inc conn est | 3398.99291372727 | 3385.40984336364 | -13.583070363637 | 3385.08184309091 | -13.9110706363631 | 3729.5363413 | 330.54342757273 | | tps exc conn est | 3399.12185727273 | 3385.52527490909 | -13.5965823636366 | 3385.22100872727 | -13.9008485454547 | 3729.7097607 | 330.58790342727 | |--+--++---+--+---+---+--| | Set 2| || | | | | | | latency average | 2.691| 2.6895 | 2 | 2.69 | 3 | 2.6827| 4| | tps inc conn est | 3719.56 | 3721.7587106 | 2.1987106 | 3720.3 | .74 | 3730.86 | 11.30| | tps exc conn est | 3719.71 | 3721.9268465 | 2.2168465 | 3720.47 | .76 | 3731.02 | 11.31| |--+--++---+--+---+---+--| > I think it may be necessary to track the current allocation size in > shared memory (with atomic increments?). Maybe decrements would need > to > be exactly accounted for, or otherwise Assert() that the value is not > negative. I don't know how expensive it'd be to have conditionals > for > each decrement, but maybe the value would only be decremented at > strategic times, like at transaction commit or backend shutdown. > -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com From 57a79b5f72af510f8c4b9ea65f5ffb4fe1fb7798 Mon Sep 17 00:00:00 2001 From: Reid Thompson 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 (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 | 107 ++ 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, 195 insertions(+) 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' + + max_total_backend_memory (integer) + + max_total_backend_memory configuration parameter + + + + +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
Re: Add tracking of backend memory allocated to pg_stat_activity
On Wed, 2022-08-31 at 12:05 -0500, Justin Pryzby wrote:
> > + 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}'
> > ,
>
> In the past, there was concern about making pg_stat_activity wider by
> adding information that's less-essential than what's been there for
> years. This is only an int64, so it's not "wide", but I wonder if
> there's another way to expose this information? Like adding backends
> to
> pg_get_backend_memory_contexts() , maybe with another view on top of
I will take a look at pg_get_backend_memory_contexts. I will also look
at the other suggestions in the thread.
> + * shown allocated in pgstat_activity when the
>
> pg_stat
Corrected,
> > replacing the * old
>
> wrapping caused extraneous stars
Corrected
> > here asit's
>
> as it's
Corrected
> errmsg() doesn't require the outside paranthesis since a couple years
> ago.
Corrected
> > > mem_allocated cast to
> add a comma before "cast" ?
Corrected
Patch with the corrections attached
--
Reid Thompson
Senior Software Engineer
Crunchy Data, Inc.
reid.thomp...@crunchydata.com
www.crunchydata.com
--
Reid Thompson
Senior Software Engineer
Crunchy Data, Inc.
reid.thomp...@crunchydata.com
www.crunchydata.com
--
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
Date: Thu, 11 Aug 2022 12:01:25 -0400
Subject: [PATCH] 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
+
+
+ backend_mem_allocated bigint
+
+
+ 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.
+
+
+
query text
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
+
Add the ability to limit the amount of memory that can be allocated to backends.
Hi Hackers,
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
https://www.postgresql.org/message-id/67bb5c15c0489cb499723b0340f16e10c22485ec.camel%40crunchydata.com
Both patches are attached.
Add GUC variable max_total_backend_memory.
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. It is intended as a resource to
help avoid the OOM killer. A backend request that would push the total
over the limit will be denied with an out of memory error causing that
backends 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 exceeding the limit. Further requests will not be allocated
until dropping below the limit. Keep this in mind when setting this
value to avoid the OOM killer. Currently, this limit does not affect
auxiliary backend processes, this list of non-affected backend
processes is open for discussion as to what should/should not be
included. Backend memory allocations are displayed in the
pg_stat_activity view.
--
Reid Thompson
Senior Software Engineer
Crunchy Data, Inc.
reid.thomp...@crunchydata.com
www.crunchydata.com
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a5cd4e44c7..caf958310a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -2079,6 +2079,32 @@ include_dir 'conf.d'
+
+ max_total_backend_memory (integer)
+
+ max_total_backend_memory configuration parameter
+
+
+
+
+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 backends 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 exceeding 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
+(backend_mem_allocated) are displayed in the
+pg_stat_activity
+view.
+
+
+
+
diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c
index 269ad2fe53..808ffe75f2 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 17a00587f8..9137a000ae 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 */
@@ -1253,3 +1255,107 @@ 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
Bug fix. autovacuum.c do_worker_start() associates memory allocations with TopMemoryContext rather than 'Autovacuum start worker (tmp)'
Hi Hackers, This patch ensures get_database_list() switches back to the memory context in use upon entry rather than returning with TopMemoryContext as the context. This will address memory allocations in autovacuum.c being associated with TopMemoryContext when they should not be. autovacuum.c do_start_worker() with current context 'Autovacuum start worker (tmp)' invokes get_database_list(). Upon return, the current context has been changed to TopMemoryContext by AtCommit_Memory() as part of an internal transaction. Further down in the do_start_worker(), pgstat_fetch_stat_dbentry() is invoked. Previously this didn't pose a issue, however recent changes altered how pgstat_fetch_stat_dbentry() is implemented. The new implementation has a branch utilizing palloc. The patch ensures these allocations are associated with the 'Autovacuum start worker (tmp)' context rather than the TopMemoryContext. Prior to the change, leaving an idle laptop PG instance running just shy of 3 days saw the autovacuum launcher process grow to 42MB with most of that growth in TopMemoryContext due to the palloc allocations issued with autovacuum worker startup. -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index b3b1afba86..92b1ef45c1 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -1941,6 +1941,13 @@ get_database_list(void) CommitTransactionCommand(); + /* + * CommitTransactionCommand returns with CurrentMemoryContext set + * to TopMemoryContext. Switch back to the context that we entered + * with. + */ + MemoryContextSwitchTo(resultcxt); + return dblist; }
Add tracking of backend memory allocated to pg_stat_activity
Hi Hackers,
Attached is a patch to
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.
--
Reid Thompson
Senior Software Engineer
Crunchy Data, Inc.
reid.thomp...@crunchydata.com
www.crunchydata.com
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
+
+
+ backend_mem_allocated bigint
+
+
+ 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.
+
+
+
query text
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..269ad2fe53 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 pgstat_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 pgstat_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 pgstat_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 pgstat_activity for the creator
Re: Patch to address creation of PgStat* contexts with null parent context
On Fri, 2022-07-29 at 11:53 +0900, Kyotaro Horiguchi wrote: > > That makes the memorycontext-tree structure unstable because > CacheMemoryContext can be created on-the-fly. > > Honestly I don't like to call CreateCacheMemoryContext in the two > functions on-the-fly. Since every process that calls > pgstat_initialize() necessarily calls pgstat_setup_memcxt() at latest > at process termination, I think we can create at least > CacheMemoryContext in pgstat_initialize(). Attached is a patch creating CacheMemoryContext() in pgstat_initialize() rather than the two previous patch locations. > Or couldn't we create the > all three contexts in the function, instead of calling > pgstat_setup_memcxt() on-the fly? You note that that pgstat_setup_memcxt() is called at latest at process termination -- was the intent to hold off on requesting memory for these two contexts until it was needed? > regards. > > -- > Kyotaro Horiguchi > NTT Open Source Software Center -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index ffd096405e..b5b69ead2b 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -532,6 +532,8 @@ pgstat_initialize(void) /* Set up a process-exit hook to clean up */ before_shmem_exit(pgstat_shutdown_hook, 0); + CreateCacheMemoryContext(); + #ifdef USE_ASSERT_CHECKING pgstat_is_initialized = true; #endif
Oversight in slab.c SlabContextCreate(), initial memory allocation size is not populated to context->mem_allocated
Hi, Both aset.c and generation.c populate mem_allocated in AllocSetContextCreateInternal(), GenerationContextCreate() respectively. aset.c /* Finally, do the type-independent part of context creation */ MemoryContextCreate((MemoryContext) set, T_AllocSetContext, , parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; return (MemoryContext) set; } generation.c /* Finally, do the type-independent part of context creation */ MemoryContextCreate((MemoryContext) set, T_GenerationContext, , parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; return (MemoryContext) set; } slab.c does not in SlabContextCreate(). Is this intentional, it seems to be an oversight to me. /* Finally, do the type-independent part of context creation */ MemoryContextCreate((MemoryContext) slab, T_SlabContext, , parent, name); return (MemoryContext) slab; } -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. reid.thomp...@crunchydata.com www.crunchydata.com
Patch to address creation of PgStat* contexts with null parent context
Hi,
There are instances where pgstat_setup_memcxt() and
pgstat_prep_pending_entry() are invoked before the CacheMemoryContext
has been created. This results in PgStat* contexts being created
without a parent context. Most easily reproduced/seen in autovacuum
worker via pgstat_setup_memcxt().
Attached is a patch to address this.
To see the issue one can add a line similar to this to the top of
MemoryContextCreate() in mcxt.c
fprintf(stderr, "pid: %d ctxname: %s parent is %s CacheMemoryContext is %s\n",
MyProcPid, name, parent == NULL ? "NULL" : "not NULL", CacheMemoryContext ==
NULL ? "NULL" : "Not NULL")
and startup postgres and grep for the above after autovacuum workers
have been invoked
...snip...
pid: 1427756 ctxname: PgStat Pending parent is NULL CacheMemoryContext is NULL
pid: 1427756 ctxname: PgStat Shared Ref parent is NULL CacheMemoryContext is
NULL
...snip...
or
startup postgres, attach gdb to postgres following child, break at
pgstat_setup_memcxt and wait for autovacuum worker to start...
...snip...
─── Source
─
384 AllocSetContextCreateInternal(MemoryContext parent,
385 const char *name,
386 Size minContextSize,
387 Size initBlockSize,
388 Size maxBlockSize)
389 {
390 int freeListIndex;
391 Size firstBlockSize;
392 AllocSet set;
393 AllocBlock block;
─── Stack
──
[0] from 0x55b7e4088b40 in AllocSetContextCreateInternal+0 at
/home/rthompso/src/git/postgres/src/backend/utils/mmgr/aset.c:389
[1] from 0x55b7e3f41c88 in pgstat_setup_memcxt+2544 at
/home/rthompso/src/git/postgres/src/backend/utils/activity/pgstat_shmem.c:979
[2] from 0x55b7e3f41c88 in pgstat_get_entry_ref+2648 at
/home/rthompso/src/git/postgres/src/backend/utils/activity/pgstat_shmem.c:410
[3] from 0x55b7e3f420ea in pgstat_get_entry_ref_locked+26 at
/home/rthompso/src/git/postgres/src/backend/utils/activity/pgstat_shmem.c:598
[4] from 0x55b7e3f3e2c4 in pgstat_report_autovac+36 at
/home/rthompso/src/git/postgres/src/backend/utils/activity/pgstat_database.c:68
[5] from 0x55b7e3e7f267 in AutoVacWorkerMain+807 at
/home/rthompso/src/git/postgres/src/backend/postmaster/autovacuum.c:1694
[6] from 0x55b7e3e7f435 in StartAutoVacWorker+133 at
/home/rthompso/src/git/postgres/src/backend/postmaster/autovacuum.c:1493
[7] from 0x55b7e3e87367 in StartAutovacuumWorker+557 at
/home/rthompso/src/git/postgres/src/backend/postmaster/postmaster.c:5539
[8] from 0x55b7e3e87367 in sigusr1_handler+935 at
/home/rthompso/src/git/postgres/src/backend/postmaster/postmaster.c:5244
[9] from 0x7fb02bca7420 in __restore_rt
[+]
─── Threads
[1] id 1174088 name postgres from 0x55b7e4088b40 in
AllocSetContextCreateInternal+0 at
/home/rthompso/src/git/postgres/src/backend/utils/mmgr/aset.c:389
─── Variables
──
arg parent = 0x0, name = 0x55b7e416f179 "PgStat Shared Ref": 80 'P',
minContextSize = 0, initBlockSize = 1024, maxBlockSize = 8192
loc firstBlockSize = , set = , block = , __FUNCTION__ = "AllocSetContextCreateInternal", __func__ =
"AllocSetContextCreateInternal"
> > > print CacheMemoryContext == NULL
$4 = 1
> > > print parent
$5 = (MemoryContext) 0x0
Thanks,
Reid
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 0d9d09c492..bf53eba660 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -102,6 +102,7 @@
#include "storage/lwlock.h"
#include "storage/pg_shmem.h"
#include "storage/shmem.h"
+#include "utils/catcache.h"
#include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/pgstat_internal.h"
@@ -1059,6 +1060,9 @@ pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, Oid
Re: PostgreSQL pollutes the file system
On Wed, 2019-03-27 at 15:07 +0100, Andreas Karlsson wrote: > [EXTERNAL SOURCE] > > > > On 3/27/19 2:51 PM, Tomas Vondra wrote: > > I think the consensus in this thread (and the previous ancient ones) is > > that it's not worth it. It's one thing to introduce new commands with the > > pg_ prefix, and it's a completely different thing to rename existing ones. > > That has inherent costs, and as Tom pointed out the burden would fall on > > people using PostgreSQL (and that's rather undesirable). > > > > I personally don't see why having commands without pg_ prefix would be > > an issue. Especially when placed in a separate directory, which eliminates > > the possibility of conflict with other commands. > > I buy that it may not be worth breaking tens of thousands of scripts to > fix this, but I disagree about it not being an issue. Most Linux > distributions add PostgreSQL's executables in to a directory which is in > the default $PATH (/usr/bin in the case of Debian). And even if it would > be installed into a separate directory there would still be a conflict > as soon as that directory is added to $PATH. > > And I think that it is also relatively easy to confuse adduser and > createuser when reading a script. Nothing about the name createuser > indicates that it will create a role in an SQL database. > > Andreas > theres nothing about createuser or adduser( useradd on my system, adduser doesn't exist on mine ) that indicates that either would/should create a user in the system either. That's what man and -h/--help are for. If you don't know what an executable does, don't invoke it until you do. That's a basic premise for any executable. reid
