agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH 2/2] doc review 25+ messages / 8 participants [nested] [flat]
* [PATCH 2/2] doc review @ 2021-03-03 22:36 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 25+ messages in thread From: Justin Pryzby @ 2021-03-03 22:36 UTC (permalink / raw) --- doc/src/sgml/libpq.sgml | 67 +++++++++++++------------- src/interfaces/libpq/fe-exec.c | 2 +- src/test/modules/test_libpq/pipeline.c | 6 +-- 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index a34ecbb144..e2865a218b 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -3130,8 +3130,9 @@ ExecStatusType PQresultStatus(const PGresult *res); <term><literal>PGRES_PIPELINE_SYNC</literal></term> <listitem> <para> - The <structname>PGresult</structname> represents a point in a - pipeline where a synchronization point has been established. + The <structname>PGresult</structname> represents a + synchronization point in pipeline mode, requested by + <xref linkend="libpq-PQsendPipeline"/>. This status occurs only when pipeline mode has been selected. </para> </listitem> @@ -3141,9 +3142,9 @@ ExecStatusType PQresultStatus(const PGresult *res); <term><literal>PGRES_PIPELINE_ABORTED</literal></term> <listitem> <para> - The <structname>PGresult</structname> represents a pipeline that's + The <structname>PGresult</structname> represents a pipeline that has received an error from the server. <function>PQgetResult</function> - must be called repeatedly, and it will return this status code, + must be called repeatedly, and each time it will return this status code until the end of the current pipeline, at which point it will return <literal>PGRES_PIPELINE_SYNC</literal> and normal processing can resume. @@ -4953,7 +4954,7 @@ int PQflush(PGconn *conn); <title>Using Pipeline Mode</title> <para> - To issue pipelines the application must switch a connection into pipeline mode. + To issue pipelines, the application must switch a connection into pipeline mode. Enter pipeline mode with <xref linkend="libpq-PQenterPipelineMode"/> or test whether pipeline mode is active with <xref linkend="libpq-PQpipelineStatus"/>. @@ -4975,7 +4976,7 @@ int PQflush(PGconn *conn); server will block trying to send results to the client from queries it has already processed. This only occurs when the client sends enough queries to fill both its output buffer and the server's receive - buffer before switching to processing input from the server, + buffer before it switches to processing input from the server, but it's hard to predict exactly when that will happen. </para> </footnote> @@ -5025,7 +5026,7 @@ int PQflush(PGconn *conn); <title>Processing Results</title> <para> - To process the result of one pipeline query, the application calls + To process the result of one query in a pipeline, the application calls <function>PQgetResult</function> repeatedly and handles each result until <function>PQgetResult</function> returns null. The result from the next query in the pipeline may then be retrieved using @@ -5057,10 +5058,10 @@ int PQflush(PGconn *conn); <type>PGresult</type> types <literal>PGRES_PIPELINE_SYNC</literal> and <literal>PGRES_PIPELINE_ABORTED</literal>. <literal>PGRES_PIPELINE_SYNC</literal> is reported exactly once for each - <function>PQsendPipeline</function> call at the corresponding point in - the result stream. + <function>PQsendPipeline</function> after retrieving results for all + queries in the pipeline. <literal>PGRES_PIPELINE_ABORTED</literal> is emitted in place of a normal - result stream result for the first error and all subsequent results + stream result for the first error and all subsequent results except <literal>PGRES_PIPELINE_SYNC</literal> and null; see <xref linkend="libpq-pipeline-errors"/>. </para> @@ -5075,7 +5076,8 @@ int PQflush(PGconn *conn); application about the query currently being processed (except that <function>PQgetResult</function> returns null to indicate that we start returning the results of next query). The application must keep track - of the order in which it sent queries and the expected results. + of the order in which it sent queries, to associate them with their + corresponding results. Applications will typically use a state machine or a FIFO queue for this. </para> @@ -5091,10 +5093,10 @@ int PQflush(PGconn *conn); </para> <para> - From the client perspective, after the client gets a - <literal>PGRES_FATAL_ERROR</literal> return from - <function>PQresultStatus</function> the pipeline is flagged as aborted. - <application>libpq</application> will report + From the client perspective, after <function>PQresultStatus</function> + returns <literal>PGRES_FATAL_ERROR</literal>, + the pipeline is flagged as aborted. + <function>PQresultStatus</function>, will report a <literal>PGRES_PIPELINE_ABORTED</literal> result for each remaining queued operation in an aborted pipeline. The result for <function>PQsendPipeline</function> is reported as @@ -5108,8 +5110,8 @@ int PQflush(PGconn *conn); </para> <para> - If the pipeline used an implicit transaction then operations that have - already executed are rolled back and operations that were queued for after + If the pipeline used an implicit transaction, then operations that have + already executed are rolled back and operations that were queued to follow the failed operation are skipped entirely. The same behaviour holds if the pipeline starts and commits a single explicit transaction (i.e. the first statement is <literal>BEGIN</literal> and the last is @@ -5145,11 +5147,11 @@ int PQflush(PGconn *conn); <para> The client application should generally maintain a queue of work - still to be dispatched and a queue of work that has been dispatched + remaining to be dispatched and a queue of work that has been dispatched but not yet had its results processed. When the socket is writable it should dispatch more work. When the socket is readable it should read results and process them, matching them up to the next entry in - its expected results queue. Based on available memory, results from + its expected results queue. Based on available memory, results from the socket should be read frequently: there's no need to wait until the pipeline end to read the results. Pipelines should be scoped to logical units of work, usually (but not necessarily) one transaction per pipeline. @@ -5191,8 +5193,8 @@ int PQflush(PGconn *conn); <listitem> <para> - Returns current pipeline mode status of the <application>libpq</application> - connection. + Returns the current pipeline mode status of the + <application>libpq</application> connection. <synopsis> PGpipelineStatus PQpipelineStatus(const PGconn *conn); </synopsis> @@ -5233,11 +5235,10 @@ PGpipelineStatus PQpipelineStatus(const PGconn *conn); <listitem> <para> The <application>libpq</application> connection is in pipeline - mode and an error has occurred while processing the current - pipeline. - The aborted flag is cleared as soon as the result - of the <function>PQsendPipeline</function> at the end of the aborted - pipeline is processed. Clients don't usually need this function to + mode and an error occurred while processing the current pipeline. + The aborted flag is cleared when <function>PQresultStatus</function> + returns PGRES_PIPELINE_SYNC at the end of the pipeline. + Clients don't usually need this function to verify aborted status, as they can tell that the pipeline is aborted from the <literal>PGRES_PIPELINE_ABORTED</literal> result code. </para> @@ -5328,7 +5329,7 @@ int PQsendPipeline(PGconn *conn); Returns 1 for success. Returns 0 if the connection is not in pipeline mode or sending a <link linkend="protocol-flow-ext-query">sync message</link> - is failed. + failed. </para> </listitem> </varlistentry> @@ -5349,7 +5350,7 @@ int PQsendPipeline(PGconn *conn); <para> Pipeline mode is most useful when the server is distant, i.e., network latency (<quote>ping time</quote>) is high, and also when many small operations - are being performed in rapid sequence. There is usually less benefit + are being performed in rapid succession. There is usually less benefit in using pipelined commands when each query takes many multiples of the client/server round-trip time to execute. A 100-statement operation run on a server 300ms round-trip-time away would take 30 seconds in network latency alone @@ -5367,7 +5368,7 @@ int PQsendPipeline(PGconn *conn); <para> Pipeline mode is not useful when information from one operation is required by the client to produce the next operation. In such cases, the client - must introduce a synchronization point and wait for a full client/server + would have to introduce a synchronization point and wait for a full client/server round-trip to get the results it needs. However, it's often possible to adjust the client design to exchange the required information server-side. Read-modify-write cycles are especially good candidates; for example: @@ -5392,10 +5393,10 @@ UPDATE mytable SET x = x + 1 WHERE id = 42; <note> <para> - The pipeline API was introduced in PostgreSQL 14, but clients using - the PostgreSQL 14 version of <application>libpq</application> can use - pipelines on server versions 7.4 and newer. Pipeline mode works on any server - that supports the v3 extended query protocol. + The pipeline API was introduced in <productname>PostgreSQL</productname> 14. + Pipeline mode is a client-side feature which doesn't require server + support, and works on any server that supports the v3 extended query + protocol. </para> </note> </sect2> diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index 7ae7c14948..d7b036a35c 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -3803,7 +3803,7 @@ pqPipelineFlush(PGconn *conn) { if ((conn->pipelineStatus == PQ_PIPELINE_OFF) || (conn->outCount >= OUTBUFFER_THRESHOLD)) - return (pqFlush(conn)); + return pqFlush(conn); return 0; } diff --git a/src/test/modules/test_libpq/pipeline.c b/src/test/modules/test_libpq/pipeline.c index f4a2bdec57..01d5a9a8ff 100644 --- a/src/test/modules/test_libpq/pipeline.c +++ b/src/test/modules/test_libpq/pipeline.c @@ -126,7 +126,7 @@ test_simple_pipeline(PGconn *conn) * process the results of as they come in. * * For a simple case we should be able to do this without interim - * processing of results since our out buffer will give us enough slush to + * processing of results since our output buffer will give us enough slush to * work with and we won't block on sending. So blocking mode is fine. */ if (PQisnonblocking(conn)) @@ -603,7 +603,7 @@ test_pipelined_insert(PGconn *conn, int n_rows) /* * Now we start inserting. We'll be sending enough data that we could fill - * our out buffer, so to avoid deadlocking we need to enter nonblocking + * our output buffer, so to avoid deadlocking we need to enter nonblocking * mode and consume input while we send more output. As results of each * query are processed we should pop them to allow processing of the next * query. There's no need to finish the pipeline before processing @@ -635,7 +635,7 @@ test_pipelined_insert(PGconn *conn, int n_rows) } /* - * Process any results, so we keep the server's out buffer free + * Process any results, so we keep the server's output buffer free * flowing and it can continue to process input */ if (FD_ISSET(sock, &input_mask)) -- 2.17.0 --oC1+HKm2/end4ao3-- ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-10-25 18:51 Reid Thompson <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Reid Thompson @ 2022-10-25 18:51 UTC (permalink / raw) To: pgsql-hackers <[email protected]>; +Cc: [email protected] patch rebased to current master -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. [email protected] www.crunchydata.com ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-10-25 18:59 Reid Thompson <[email protected]> parent: Reid Thompson <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Reid Thompson @ 2022-10-25 18:59 UTC (permalink / raw) To: [email protected]; +Cc: [email protected] 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. [email protected] www.crunchydata.com Attachments: [text/x-patch] 0001-Add-tracking-of-backend-memory-allocated-to-pg_stat_.patch (28.5K, ../../[email protected]/2-0001-Add-tracking-of-backend-memory-allocated-to-pg_stat_.patch) download | inline diff: From ab654a48ec7bfbc3bc377c5757a04f1756e72e79 Mon Sep 17 00:00:00 2001 From: Reid Thompson <[email protected]> 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 </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>backend_mem_allocated</structfield> <type>bigint</type> + </para> + <para> + The byte count of memory allocated to this backend. Dynamic shared memory + allocations are included only in the value displayed for the backend that + created them, they are not included in the value for backends that are + attached to them to avoid double counting. + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>query</structfield> <type>text</type> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 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(request_size); + } +#else + pgstat_report_backend_mem_allocated_decrease(*mapped_size); + pgstat_report_backend_mem_allocated_increase(request_size); +#endif + } *mapped_address = address; *mapped_size = request_size; close(fd); @@ -537,6 +575,14 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_backend_mem_allocated_decrease(*mapped_size); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && shmctl(ident, IPC_RMID, NULL) < 0) @@ -584,6 +630,13 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_backend_mem_allocated_increase(request_size); *mapped_address = address; *mapped_size = request_size; @@ -652,6 +705,13 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return false; } + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_backend_mem_allocated_decrease(*mapped_size); *impl_private = NULL; *mapped_address = NULL; *mapped_size = 0; @@ -768,6 +828,12 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return false; } + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_backend_mem_allocated_increase(info.RegionSize); *mapped_address = address; *mapped_size = info.RegionSize; *impl_private = hmap; @@ -812,6 +878,13 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + pgstat_report_backend_mem_allocated_decrease(*mapped_size); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && unlink(name) != 0) @@ -933,6 +1006,13 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_backend_mem_allocated_increase(request_size); *mapped_address = address; *mapped_size = request_size; diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index 1146a6c33c..5c33824eb8 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -49,6 +49,8 @@ int pgstat_track_activity_query_size = 1024; /* exposed so that backend_progress.c can access it */ PgBackendStatus *MyBEEntry = NULL; +/* Memory allocated to this backend prior to pgstats initialization */ +uint64 backend_mem_allocated = 0; static PgBackendStatus *BackendStatusArray = NULL; static char *BackendAppnameBuffer = NULL; @@ -400,6 +402,13 @@ pgstat_bestart(void) lbeentry.st_progress_command_target = InvalidOid; lbeentry.st_query_id = UINT64CONST(0); + /* + * Move sum of memory allocated prior to pgstats initialization to pgstats + * and zero the local variable. + */ + lbeentry.backend_mem_allocated = backend_mem_allocated; + backend_mem_allocated = 0; + /* * we don't zero st_progress_param here to save cycles; nobody should * examine it until st_progress_command has been set to something other @@ -1191,3 +1200,99 @@ pgstat_clip_activity(const char *raw_activity) return activity; } + +/* -------- + * pgstat_report_backend_mem_allocated_increase() - + * + * Called to report increase in memory allocated for this backend + * -------- + */ +void +pgstat_report_backend_mem_allocated_increase(uint64 allocation) +{ + volatile PgBackendStatus *beentry = MyBEEntry; + + if (!beentry || !pgstat_track_activities) + { + /* + * Account for memory before pgstats is initialized. This will be + * migrated to pgstats on initialization. + */ + backend_mem_allocated += allocation; + + return; + } + + /* + * Update my status entry, following the protocol of bumping + * st_changecount before and after. We use a volatile pointer here to + * ensure the compiler doesn't try to get cute. + */ + PGSTAT_BEGIN_WRITE_ACTIVITY(beentry); + beentry->backend_mem_allocated += allocation; + PGSTAT_END_WRITE_ACTIVITY(beentry); +} + +/* -------- + * pgstat_report_backend_mem_allocated_decrease() - + * + * Called to report decrease in memory allocated for this backend + * -------- + */ +void +pgstat_report_backend_mem_allocated_decrease(uint64 deallocation) +{ + volatile PgBackendStatus *beentry = MyBEEntry; + + /* + * Cases may occur where shared memory from a previous postmaster + * invocation still exist. These are cleaned up at startup by + * dsm_cleanup_using_control_segment. Limit decreasing memory allocated to + * zero in case no corresponding prior increase exists or decrease has + * already been accounted for. + */ + + if (!beentry || !pgstat_track_activities) + { + /* + * Account for memory before pgstats is initialized. This will be + * migrated to pgstats on initialization. Do not allow + * backend_mem_allocated to go below zero. If pgstats has not been + * initialized, we are in startup and we set backend_mem_allocated to + * zero in cases where it would go negative and skip generating an + * ereport. + */ + if (deallocation > backend_mem_allocated) + backend_mem_allocated = 0; + else + backend_mem_allocated -= deallocation; + + return; + } + + /* + * Do not allow backend_mem_allocated to go below zero. ereport if we + * would have. There's no need for a lock around the read here as it's + * being referenced from the same backend which means that there shouldn't + * be concurrent writes. We want to generate an ereport in these cases. + */ + if (deallocation > beentry->backend_mem_allocated) + { + ereport(LOG, errmsg("decrease reduces reported backend memory allocated below zero; setting reported to 0")); + + /* + * Overwrite deallocation with current backend_mem_allocated so we end + * up at zero. + */ + deallocation = beentry->backend_mem_allocated; + } + + /* + * Update my status entry, following the protocol of bumping + * st_changecount before and after. We use a volatile pointer here to + * ensure the compiler doesn't try to get cute. + */ + PGSTAT_BEGIN_WRITE_ACTIVITY(beentry); + beentry->backend_mem_allocated -= deallocation; + PGSTAT_END_WRITE_ACTIVITY(beentry); +} diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 96bffc0f2a..692ed1df18 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 = pgstat_fetch_stat_numbackends(); int curr_backend; int pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0); @@ -609,6 +609,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) else nulls[16] = true; + values[30] = UInt64GetDatum(beentry->backend_mem_allocated); + /* Values only available to role member or pg_read_all_stats */ if (HAS_PGSTAT_PERMISSIONS(beentry->st_userid)) { diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index db402e3a41..f6d1333f3d 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -47,6 +47,7 @@ #include "postgres.h" #include "port/pg_bitutils.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -521,6 +522,7 @@ AllocSetContextCreateInternal(MemoryContext parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; + pgstat_report_backend_mem_allocated_increase(firstBlockSize); return (MemoryContext) set; } @@ -543,6 +545,7 @@ AllocSetReset(MemoryContext context) AllocSet set = (AllocSet) context; AllocBlock block; Size keepersize PG_USED_FOR_ASSERTS_ONLY; + uint64 deallocation = 0; AssertArg(AllocSetIsValid(set)); @@ -585,6 +588,7 @@ AllocSetReset(MemoryContext context) { /* Normal case, release the block */ context->mem_allocated -= block->endptr - ((char *) block); + deallocation += block->endptr - ((char *) block); #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -595,6 +599,7 @@ AllocSetReset(MemoryContext context) } Assert(context->mem_allocated == keepersize); + pgstat_report_backend_mem_allocated_decrease(deallocation); /* Reset block size allocation sequence, too */ set->nextBlockSize = set->initBlockSize; @@ -613,6 +618,7 @@ AllocSetDelete(MemoryContext context) AllocSet set = (AllocSet) context; AllocBlock block = set->blocks; Size keepersize PG_USED_FOR_ASSERTS_ONLY; + uint64 deallocation = 0; AssertArg(AllocSetIsValid(set)); @@ -651,11 +657,13 @@ AllocSetDelete(MemoryContext context) freelist->first_free = (AllocSetContext *) oldset->header.nextchild; freelist->num_free--; + deallocation += oldset->header.mem_allocated; /* All that remains is to free the header/initial block */ free(oldset); } Assert(freelist->num_free == 0); + pgstat_report_backend_mem_allocated_decrease(deallocation); } /* Now add the just-deleted context to the freelist. */ @@ -672,7 +680,10 @@ AllocSetDelete(MemoryContext context) AllocBlock next = block->next; if (block != set->keeper) + { context->mem_allocated -= block->endptr - ((char *) block); + deallocation += block->endptr - ((char *) block); + } #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -685,6 +696,8 @@ AllocSetDelete(MemoryContext context) } Assert(context->mem_allocated == keepersize); + pgstat_report_backend_mem_allocated_decrease(deallocation + + context->mem_allocated); /* Finally, free the context header, including the keeper block */ free(set); @@ -734,6 +747,7 @@ AllocSetAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_backend_mem_allocated_increase(blksize); block->aset = set; block->freeptr = block->endptr = ((char *) block) + blksize; @@ -944,6 +958,7 @@ AllocSetAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_backend_mem_allocated_increase(blksize); block->aset = set; block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ; @@ -1043,6 +1058,7 @@ AllocSetFree(void *pointer) block->next->prev = block->prev; set->header.mem_allocated -= block->endptr - ((char *) block); + pgstat_report_backend_mem_allocated_decrease(block->endptr - ((char *) block)); #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -1173,7 +1189,9 @@ AllocSetRealloc(void *pointer, Size size) /* updated separately, not to underflow when (oldblksize > blksize) */ set->header.mem_allocated -= oldblksize; + pgstat_report_backend_mem_allocated_decrease(oldblksize); set->header.mem_allocated += blksize; + pgstat_report_backend_mem_allocated_increase(blksize); block->freeptr = block->endptr = ((char *) block) + blksize; diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c index 4cb75f493f..7bb9175f6d 100644 --- a/src/backend/utils/mmgr/generation.c +++ b/src/backend/utils/mmgr/generation.c @@ -37,6 +37,7 @@ #include "lib/ilist.h" #include "port/pg_bitutils.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -267,6 +268,7 @@ GenerationContextCreate(MemoryContext parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; + pgstat_report_backend_mem_allocated_increase(firstBlockSize); return (MemoryContext) set; } @@ -283,6 +285,7 @@ GenerationReset(MemoryContext context) { GenerationContext *set = (GenerationContext *) context; dlist_mutable_iter miter; + uint64 deallocation = 0; AssertArg(GenerationIsValid(set)); @@ -305,9 +308,14 @@ GenerationReset(MemoryContext context) if (block == set->keeper) GenerationBlockMarkEmpty(block); else + { + deallocation += block->blksize; GenerationBlockFree(set, block); + } } + pgstat_report_backend_mem_allocated_decrease(deallocation); + /* set it so new allocations to make use of the keeper block */ set->block = set->keeper; @@ -328,6 +336,9 @@ GenerationDelete(MemoryContext context) { /* Reset to release all releasable GenerationBlocks */ GenerationReset(context); + + pgstat_report_backend_mem_allocated_decrease(context->mem_allocated); + /* And free the context header and keeper block */ free(context); } @@ -374,6 +385,7 @@ GenerationAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_backend_mem_allocated_increase(blksize); /* block with a single (used) chunk */ block->context = set; @@ -477,6 +489,7 @@ GenerationAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_backend_mem_allocated_increase(blksize); /* initialize the new block */ GenerationBlockInit(set, block, blksize); @@ -726,6 +739,8 @@ GenerationFree(void *pointer) dlist_delete(&block->node); set->header.mem_allocated -= block->blksize; + pgstat_report_backend_mem_allocated_decrease(block->blksize); + free(block); } diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c index 1a0b28f9ea..efdc2736c1 100644 --- a/src/backend/utils/mmgr/slab.c +++ b/src/backend/utils/mmgr/slab.c @@ -53,6 +53,7 @@ #include "postgres.h" #include "lib/ilist.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -238,6 +239,12 @@ SlabContextCreate(MemoryContext parent, parent, name); + /* + * If SlabContextCreate is updated to add headerSize to + * context->mem_allocated, then update here and SlabDelete appropriately + */ + pgstat_report_backend_mem_allocated_increase(headerSize); + return (MemoryContext) slab; } @@ -253,6 +260,7 @@ SlabReset(MemoryContext context) { SlabContext *slab = (SlabContext *) context; int i; + uint64 deallocation = 0; AssertArg(SlabIsValid(slab)); @@ -278,9 +286,11 @@ SlabReset(MemoryContext context) free(block); slab->nblocks--; context->mem_allocated -= slab->blockSize; + deallocation += slab->blockSize; } } + pgstat_report_backend_mem_allocated_decrease(deallocation); slab->minFreeChunks = 0; Assert(slab->nblocks == 0); @@ -294,8 +304,17 @@ SlabReset(MemoryContext context) void SlabDelete(MemoryContext context) { + /* + * Until header allocation is included in context->mem_allocated, cast to + * slab and decrement the headerSize + */ + SlabContext *slab = castNode(SlabContext, context); + /* Reset to release all the SlabBlocks */ SlabReset(context); + + pgstat_report_backend_mem_allocated_decrease(slab->headerSize); + /* And free the context header */ free(context); } @@ -364,6 +383,7 @@ SlabAlloc(MemoryContext context, Size size) slab->minFreeChunks = slab->chunksPerBlock; slab->nblocks += 1; context->mem_allocated += slab->blockSize; + pgstat_report_backend_mem_allocated_increase(slab->blockSize); } /* grab the block from the freelist (even the new block is there) */ @@ -537,6 +557,7 @@ SlabFree(void *pointer) free(block); slab->nblocks--; slab->header.mem_allocated -= slab->blockSize; + pgstat_report_backend_mem_allocated_decrease(slab->blockSize); } else dlist_push_head(&slab->freelist[block->nfree], &block->node); diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 62a5b8e655..737a7c5034 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5347,9 +5347,9 @@ proname => 'pg_stat_get_activity', prorows => '100', proisstrict => 'f', proretset => 't', provolatile => 's', proparallel => 'r', prorettype => 'record', proargtypes => 'int4', - proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8}', - proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id}', + proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8,int8}', + proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id,backend_mem_allocated}', prosrc => 'pg_stat_get_activity' }, { oid => '3318', descr => 'statistics: information about progress of backends running maintenance command', diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h index b582b46e9f..d59a1d50f6 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_mem_allocated; } PgBackendStatus; @@ -313,7 +316,9 @@ extern const char *pgstat_get_backend_current_activity(int pid, bool checkUser); extern const char *pgstat_get_crashed_backend_activity(int pid, char *buffer, int buflen); extern uint64 pgstat_get_my_query_id(void); - +extern void pgstat_report_backend_mem_allocated_increase(uint64 allocation); +extern void pgstat_report_backend_mem_allocated_decrease(uint64 deallocation); +extern uint64 pgstat_get_all_backend_memory_allocated(void); /* ---------- * Support functions for the SQL-callable functions to diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index bfcd8ac9a0..36947c636d 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1752,9 +1752,10 @@ pg_stat_activity| SELECT s.datid, s.backend_xid, s.backend_xmin, s.query_id, + s.backend_mem_allocated, s.query, s.backend_type - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, backend_mem_allocated) LEFT JOIN pg_database d ON ((s.datid = d.oid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_all_indexes| SELECT c.oid AS relid, @@ -1869,7 +1870,7 @@ pg_stat_gssapi| SELECT s.pid, s.gss_auth AS gss_authenticated, s.gss_princ AS principal, s.gss_enc AS encrypted - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, backend_mem_allocated) WHERE (s.client_port IS NOT NULL); pg_stat_progress_analyze| SELECT s.pid, s.datid, @@ -2050,7 +2051,7 @@ pg_stat_replication| SELECT s.pid, w.sync_priority, w.sync_state, w.reply_time - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, backend_mem_allocated) JOIN pg_stat_get_wal_senders() w(pid, state, sent_lsn, write_lsn, flush_lsn, replay_lsn, write_lag, flush_lag, replay_lag, sync_priority, sync_state, reply_time) ON ((s.pid = w.pid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_replication_slots| SELECT s.slot_name, @@ -2084,7 +2085,7 @@ pg_stat_ssl| SELECT s.pid, s.ssl_client_dn AS client_dn, s.ssl_client_serial AS client_serial, s.ssl_issuer_dn AS issuer_dn - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, backend_mem_allocated) WHERE (s.client_port IS NOT NULL); pg_stat_subscription| SELECT su.oid AS subid, su.subname, -- 2.25.1 ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-04 10:06 Drouvot, Bertrand <[email protected]> parent: Reid Thompson <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Drouvot, Bertrand @ 2022-11-04 10:06 UTC (permalink / raw) To: [email protected]; [email protected] Hi, On 10/25/22 8:59 PM, Reid Thompson wrote: > On Tue, 2022-10-25 at 14:51 -0400, Reid Thompson wrote: >> patch rebased to current master >> > actually attach the patch > It looks like the patch does not apply anymore since b1099eca8f. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-04 12:56 Reid Thompson <[email protected]> parent: Drouvot, Bertrand <[email protected]> 0 siblings, 2 replies; 25+ messages in thread From: Reid Thompson @ 2022-11-04 12:56 UTC (permalink / raw) To: [email protected]; +Cc: [email protected]; Drouvot, Bertrand <[email protected]> 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. [email protected] www.crunchydata.com Attachments: [text/x-patch] 0001-Add-tracking-of-backend-memory-allocated-to-pg_stat_.patch (28.6K, ../../[email protected]/2-0001-Add-tracking-of-backend-memory-allocated-to-pg_stat_.patch) download | inline diff: From a8de5d29c0c6f10962181926a49ad4fec1e52bd1 Mon Sep 17 00:00:00 2001 From: Reid Thompson <[email protected]> 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 </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>backend_allocated_bytes</structfield> <type>bigint</type> + </para> + <para> + The byte count of memory allocated to this backend. Dynamic shared memory + allocations are included only in the value displayed for the backend that + created them, they are not included in the value for backends that are + attached to them to avoid double counting. + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>query</structfield> <type>text</type> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 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_decrease(*mapped_size); + pgstat_report_backend_allocated_bytes_increase(request_size); + } +#else + pgstat_report_backend_allocated_bytes_decrease(*mapped_size); + pgstat_report_backend_allocated_bytes_increase(request_size); +#endif + } *mapped_address = address; *mapped_size = request_size; close(fd); @@ -537,6 +576,14 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_backend_allocated_bytes_decrease(*mapped_size); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && shmctl(ident, IPC_RMID, NULL) < 0) @@ -584,6 +631,13 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_backend_allocated_bytes_increase(request_size); *mapped_address = address; *mapped_size = request_size; @@ -652,6 +706,13 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return false; } + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_backend_allocated_bytes_decrease(*mapped_size); *impl_private = NULL; *mapped_address = NULL; *mapped_size = 0; @@ -768,6 +829,12 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return false; } + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_backend_allocated_bytes_increase(info.RegionSize); *mapped_address = address; *mapped_size = info.RegionSize; *impl_private = hmap; @@ -812,6 +879,13 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + pgstat_report_backend_allocated_bytes_decrease(*mapped_size); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && unlink(name) != 0) @@ -933,6 +1007,13 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_backend_allocated_bytes_increase(request_size); *mapped_address = address; *mapped_size = request_size; diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index 1146a6c33c..30a89e899a 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -49,6 +49,8 @@ int pgstat_track_activity_query_size = 1024; /* exposed so that backend_progress.c can access it */ PgBackendStatus *MyBEEntry = NULL; +/* Memory allocated to this backend prior to pgstats initialization */ +uint64 backend_allocated_bytes = 0; static PgBackendStatus *BackendStatusArray = NULL; static char *BackendAppnameBuffer = NULL; @@ -400,6 +402,13 @@ pgstat_bestart(void) lbeentry.st_progress_command_target = InvalidOid; lbeentry.st_query_id = UINT64CONST(0); + /* + * Move sum of memory allocated prior to pgstats initialization to pgstats + * and zero the local variable. + */ + lbeentry.backend_allocated_bytes = backend_allocated_bytes; + backend_allocated_bytes = 0; + /* * we don't zero st_progress_param here to save cycles; nobody should * examine it until st_progress_command has been set to something other @@ -1191,3 +1200,99 @@ pgstat_clip_activity(const char *raw_activity) return activity; } + +/* -------- + * 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); +} + +/* -------- + * pgstat_report_backend_allocated_bytes_decrease() - + * + * Called to report decrease in memory allocated for this backend + * -------- + */ +void +pgstat_report_backend_allocated_bytes_decrease(uint64 deallocation) +{ + volatile PgBackendStatus *beentry = MyBEEntry; + + /* + * Cases may occur where shared memory from a previous postmaster + * invocation still exist. These are cleaned up at startup by + * dsm_cleanup_using_control_segment. Limit decreasing memory allocated to + * zero in case no corresponding prior increase exists or decrease has + * already been accounted for. + */ + + if (!beentry || !pgstat_track_activities) + { + /* + * Account for memory before pgstats is initialized. This will be + * migrated to pgstats on initialization. Do not allow + * backend_allocated_bytes to go below zero. If pgstats has not been + * initialized, we are in startup and we set backend_allocated_bytes + * to zero in cases where it would go negative and skip generating an + * ereport. + */ + if (deallocation > backend_allocated_bytes) + backend_allocated_bytes = 0; + else + backend_allocated_bytes -= deallocation; + + return; + } + + /* + * 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")); + + /* + * Overwrite deallocation with current backend_allocated_bytes so we + * end up at zero. + */ + deallocation = beentry->backend_allocated_bytes; + } + + /* + * 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 -= deallocation; + PGSTAT_END_WRITE_ACTIVITY(beentry); +} 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 = pgstat_fetch_stat_numbackends(); int curr_backend; int pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0); @@ -609,6 +609,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) else nulls[16] = true; + values[30] = UInt64GetDatum(beentry->backend_allocated_bytes); + /* Values only available to role member or pg_read_all_stats */ if (HAS_PGSTAT_PERMISSIONS(beentry->st_userid)) { diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index b6a8bbcd59..81e62e4981 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -47,6 +47,7 @@ #include "postgres.h" #include "port/pg_bitutils.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -521,6 +522,7 @@ AllocSetContextCreateInternal(MemoryContext parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; + pgstat_report_backend_allocated_bytes_increase(firstBlockSize); return (MemoryContext) set; } @@ -543,6 +545,7 @@ AllocSetReset(MemoryContext context) AllocSet set = (AllocSet) context; AllocBlock block; Size keepersize PG_USED_FOR_ASSERTS_ONLY; + uint64 deallocation = 0; Assert(AllocSetIsValid(set)); @@ -585,6 +588,7 @@ AllocSetReset(MemoryContext context) { /* Normal case, release the block */ context->mem_allocated -= block->endptr - ((char *) block); + deallocation += block->endptr - ((char *) block); #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -595,6 +599,7 @@ AllocSetReset(MemoryContext context) } Assert(context->mem_allocated == keepersize); + pgstat_report_backend_allocated_bytes_decrease(deallocation); /* Reset block size allocation sequence, too */ set->nextBlockSize = set->initBlockSize; @@ -613,6 +618,7 @@ AllocSetDelete(MemoryContext context) AllocSet set = (AllocSet) context; AllocBlock block = set->blocks; Size keepersize PG_USED_FOR_ASSERTS_ONLY; + uint64 deallocation = 0; Assert(AllocSetIsValid(set)); @@ -651,11 +657,13 @@ AllocSetDelete(MemoryContext context) freelist->first_free = (AllocSetContext *) oldset->header.nextchild; freelist->num_free--; + deallocation += oldset->header.mem_allocated; /* All that remains is to free the header/initial block */ free(oldset); } Assert(freelist->num_free == 0); + pgstat_report_backend_allocated_bytes_decrease(deallocation); } /* Now add the just-deleted context to the freelist. */ @@ -672,7 +680,10 @@ AllocSetDelete(MemoryContext context) AllocBlock next = block->next; if (block != set->keeper) + { context->mem_allocated -= block->endptr - ((char *) block); + deallocation += block->endptr - ((char *) block); + } #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -685,6 +696,8 @@ AllocSetDelete(MemoryContext context) } Assert(context->mem_allocated == keepersize); + pgstat_report_backend_allocated_bytes_decrease(deallocation + + context->mem_allocated); /* Finally, free the context header, including the keeper block */ free(set); @@ -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)); @@ -1173,7 +1189,9 @@ AllocSetRealloc(void *pointer, Size size) /* updated separately, not to underflow when (oldblksize > blksize) */ set->header.mem_allocated -= oldblksize; + pgstat_report_backend_allocated_bytes_decrease(oldblksize); set->header.mem_allocated += blksize; + pgstat_report_backend_allocated_bytes_increase(blksize); block->freeptr = block->endptr = ((char *) block) + blksize; diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c index b432a92be3..df3007edfb 100644 --- a/src/backend/utils/mmgr/generation.c +++ b/src/backend/utils/mmgr/generation.c @@ -37,6 +37,7 @@ #include "lib/ilist.h" #include "port/pg_bitutils.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -267,6 +268,7 @@ GenerationContextCreate(MemoryContext parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; + pgstat_report_backend_allocated_bytes_increase(firstBlockSize); return (MemoryContext) set; } @@ -283,6 +285,7 @@ GenerationReset(MemoryContext context) { GenerationContext *set = (GenerationContext *) context; dlist_mutable_iter miter; + uint64 deallocation = 0; Assert(GenerationIsValid(set)); @@ -305,9 +308,14 @@ GenerationReset(MemoryContext context) if (block == set->keeper) GenerationBlockMarkEmpty(block); else + { + deallocation += block->blksize; GenerationBlockFree(set, block); + } } + pgstat_report_backend_allocated_bytes_decrease(deallocation); + /* set it so new allocations to make use of the keeper block */ set->block = set->keeper; @@ -328,6 +336,9 @@ GenerationDelete(MemoryContext context) { /* Reset to release all releasable GenerationBlocks */ GenerationReset(context); + + pgstat_report_backend_allocated_bytes_decrease(context->mem_allocated); + /* And free the context header and keeper block */ free(context); } @@ -374,6 +385,7 @@ GenerationAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_backend_allocated_bytes_increase(blksize); /* block with a single (used) chunk */ block->context = set; @@ -477,6 +489,7 @@ GenerationAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_backend_allocated_bytes_increase(blksize); /* initialize the new block */ GenerationBlockInit(set, block, blksize); @@ -726,6 +739,8 @@ GenerationFree(void *pointer) dlist_delete(&block->node); set->header.mem_allocated -= block->blksize; + pgstat_report_backend_allocated_bytes_decrease(block->blksize); + free(block); } diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c index 6df0839b6a..532c038973 100644 --- a/src/backend/utils/mmgr/slab.c +++ b/src/backend/utils/mmgr/slab.c @@ -53,6 +53,7 @@ #include "postgres.h" #include "lib/ilist.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -238,6 +239,12 @@ SlabContextCreate(MemoryContext parent, parent, name); + /* + * If SlabContextCreate is updated to add headerSize to + * context->mem_allocated, then update here and SlabDelete appropriately + */ + pgstat_report_backend_allocated_bytes_increase(headerSize); + return (MemoryContext) slab; } @@ -253,6 +260,7 @@ SlabReset(MemoryContext context) { SlabContext *slab = (SlabContext *) context; int i; + uint64 deallocation = 0; Assert(SlabIsValid(slab)); @@ -278,9 +286,11 @@ SlabReset(MemoryContext context) free(block); slab->nblocks--; context->mem_allocated -= slab->blockSize; + deallocation += slab->blockSize; } } + pgstat_report_backend_allocated_bytes_decrease(deallocation); slab->minFreeChunks = 0; Assert(slab->nblocks == 0); @@ -294,8 +304,17 @@ SlabReset(MemoryContext context) void SlabDelete(MemoryContext context) { + /* + * Until header allocation is included in context->mem_allocated, cast to + * slab and decrement the headerSize + */ + SlabContext *slab = castNode(SlabContext, context); + /* Reset to release all the SlabBlocks */ SlabReset(context); + + pgstat_report_backend_allocated_bytes_decrease(slab->headerSize); + /* And free the context header */ free(context); } @@ -364,6 +383,7 @@ SlabAlloc(MemoryContext context, Size size) slab->minFreeChunks = slab->chunksPerBlock; slab->nblocks += 1; context->mem_allocated += slab->blockSize; + pgstat_report_backend_allocated_bytes_increase(slab->blockSize); } /* grab the block from the freelist (even the new block is there) */ @@ -537,6 +557,7 @@ SlabFree(void *pointer) free(block); slab->nblocks--; slab->header.mem_allocated -= slab->blockSize; + pgstat_report_backend_allocated_bytes_decrease(slab->blockSize); } else dlist_push_head(&slab->freelist[block->nfree], &block->node); diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 20f5aa56ea..1c37f7db5d 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5347,9 +5347,9 @@ proname => 'pg_stat_get_activity', prorows => '100', proisstrict => 'f', proretset => 't', provolatile => 's', proparallel => 'r', prorettype => 'record', proargtypes => 'int4', - proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8}', - proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id}', + proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8,int8}', + proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id,backend_allocated_bytes}', prosrc => 'pg_stat_get_activity' }, { oid => '3318', descr => 'statistics: information about progress of backends running maintenance command', diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h index 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; @@ -313,7 +316,9 @@ extern const char *pgstat_get_backend_current_activity(int pid, bool checkUser); extern const char *pgstat_get_crashed_backend_activity(int pid, char *buffer, int buflen); extern uint64 pgstat_get_my_query_id(void); - +extern void pgstat_report_backend_allocated_bytes_increase(uint64 allocation); +extern void pgstat_report_backend_allocated_bytes_decrease(uint64 deallocation); +extern uint64 pgstat_get_all_backend_memory_allocated(void); /* ---------- * Support functions for the SQL-callable functions to diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 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 - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, backend_allocated_bytes) LEFT JOIN pg_database d ON ((s.datid = d.oid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_all_indexes| SELECT c.oid AS relid, @@ -1871,7 +1872,7 @@ pg_stat_gssapi| SELECT s.pid, s.gss_auth AS gss_authenticated, s.gss_princ AS principal, s.gss_enc AS encrypted - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, backend_allocated_bytes) WHERE (s.client_port IS NOT NULL); pg_stat_progress_analyze| SELECT s.pid, s.datid, @@ -2052,7 +2053,7 @@ pg_stat_replication| SELECT s.pid, w.sync_priority, w.sync_state, w.reply_time - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, backend_allocated_bytes) JOIN pg_stat_get_wal_senders() w(pid, state, sent_lsn, write_lsn, flush_lsn, replay_lsn, write_lag, flush_lag, replay_lag, sync_priority, sync_state, reply_time) ON ((s.pid = w.pid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_replication_slots| SELECT s.slot_name, @@ -2086,7 +2087,7 @@ pg_stat_ssl| SELECT s.pid, s.ssl_client_dn AS client_dn, s.ssl_client_serial AS client_serial, s.ssl_issuer_dn AS issuer_dn - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, backend_allocated_bytes) WHERE (s.client_port IS NOT NULL); pg_stat_subscription| SELECT su.oid AS subid, su.subname, -- 2.25.1 ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-05 02:41 Andres Freund <[email protected]> parent: Reid Thompson <[email protected]> 1 sibling, 1 reply; 25+ messages in thread From: Andres Freund @ 2022-11-05 02:41 UTC (permalink / raw) To: Reid Thompson <[email protected]>; +Cc: [email protected]; Drouvot, Bertrand <[email protected]> Hi, On 2022-11-04 08:56:13 -0400, Reid Thompson wrote: > From a8de5d29c0c6f10962181926a49ad4fec1e52bd1 Mon Sep 17 00:00:00 2001 > From: Reid Thompson <[email protected]> > 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. 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. > @@ -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! 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. > +void > +pgstat_report_backend_allocated_bytes_decrease(uint64 deallocation) > +{ > + volatile PgBackendStatus *beentry = MyBEEntry; > + > + /* > + * Cases may occur where shared memory from a previous postmaster > + * invocation still exist. These are cleaned up at startup by > + * dsm_cleanup_using_control_segment. Limit decreasing memory allocated to > + * zero in case no corresponding prior increase exists or decrease has > + * already been accounted for. > + */ I don't really follow - postmaster won't ever have a backend status array, so how would they be tracked here? Greetings, Andres Freund ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-07 21:17 Melanie Plageman <[email protected]> parent: Reid Thompson <[email protected]> 1 sibling, 1 reply; 25+ messages in thread From: Melanie Plageman @ 2022-11-07 21:17 UTC (permalink / raw) To: Reid Thompson <[email protected]>; +Cc: [email protected]; Drouvot, Bertrand <[email protected]> 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 <[email protected]> > 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 > </para></entry> > </row> > > + <row> > + <entry role="catalog_table_entry"><para role="column_definition"> > + <structfield>backend_allocated_bytes</structfield> <type>bigint</type> > + </para> > + <para> > + The byte count of memory allocated to this backend. Dynamic shared memory > + allocations are included only in the value displayed for the backend that > + created them, they are not included in the value for backends that are > + attached to them to avoid double counting. > + </para></entry> > + </row> > + It doesn't seem like you need the backend_ prefix in the view since that is implied by it being in pg_stat_activity. 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. 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(). > +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); > +} > + > +/* -------- > + * pgstat_report_backend_allocated_bytes_decrease() - > + * > + * Called to report decrease in memory allocated for this backend > + * -------- > + */ > +void > +pgstat_report_backend_allocated_bytes_decrease(uint64 deallocation) > +{ > + volatile PgBackendStatus *beentry = MyBEEntry; > + > + /* > + * Cases may occur where shared memory from a previous postmaster > + * invocation still exist. These are cleaned up at startup by > + * dsm_cleanup_using_control_segment. Limit decreasing memory allocated to > + * zero in case no corresponding prior increase exists or decrease has > + * already been accounted for. > + */ > + > + if (!beentry || !pgstat_track_activities) > + { > + /* > + * Account for memory before pgstats is initialized. This will be > + * migrated to pgstats on initialization. Do not allow > + * backend_allocated_bytes to go below zero. If pgstats has not been > + * initialized, we are in startup and we set backend_allocated_bytes > + * to zero in cases where it would go negative and skip generating an > + * ereport. > + */ > + if (deallocation > backend_allocated_bytes) > + backend_allocated_bytes = 0; > + else > + backend_allocated_bytes -= deallocation; > + > + return; > + } > + > + /* > + * 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. > 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 = pgstat_fetch_stat_numbackends(); > int curr_backend; > int pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0); > @@ -609,6 +609,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) > else > nulls[16] = true; > > + 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 +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. > @@ -313,7 +316,9 @@ extern const char *pgstat_get_backend_current_activity(int pid, bool checkUser); > extern const char *pgstat_get_crashed_backend_activity(int pid, char *buffer, > int buflen); > extern uint64 pgstat_get_my_query_id(void); > - > +extern void pgstat_report_backend_allocated_bytes_increase(uint64 allocation); > +extern void pgstat_report_backend_allocated_bytes_decrease(uint64 deallocation); > +extern uint64 pgstat_get_all_backend_memory_allocated(void); > > /* ---------- > * Support functions for the SQL-callable functions to > diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out > index 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. - Melanie [1] https://www.postgresql.org/message-id/20221105024146.xxlbtsxh2niyz2fu%40awork3.anarazel.de ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-09 13:54 Reid Thompson <[email protected]> parent: Andres Freund <[email protected]> 0 siblings, 2 replies; 25+ messages in thread From: Reid Thompson @ 2022-11-09 13:54 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: [email protected]; [email protected]; Drouvot, Bertrand <[email protected]> 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 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. > > > +void > > +pgstat_report_backend_allocated_bytes_decrease(uint64 > > deallocation) > > +{ > > +       volatile PgBackendStatus *beentry = MyBEEntry; > > + > > +       /* > > +       * Cases may occur where shared memory from a previous postmaster > > +       * invocation still exist. These are cleaned up at startup by > > +       * dsm_cleanup_using_control_segment. Limit decreasing memory allocated to > > +       * zero in case no corresponding prior increase exists or decrease has > > +       * already been accounted for. > > +       */ > > I don't really follow - postmaster won't ever have a backend status > array, so how would they be tracked here? On startup, a check is made for leftover dsm control segments in the DataDir. It appears possible that in certain situations on startup we may find and destroy stale segments and thus decrement the allocation variable. I based this off of: /ipc/dsm.c dsm_postmaster_startup: 150 dsm_postmaster_startup(PGShmemHeader *shim) { ...snip... 158 /* 159 ¦* If we're using the mmap implementations, clean up any leftovers. 160 ¦* Cleanup isn't needed on Windows, and happens earlier in startup for 161 ¦* POSIX and System V shared memory, via a direct call to 162 ¦* dsm_cleanup_using_control_segment. 163 ¦*/ ...snip... } dsm_cleanup_using_control_segment: 206 /* 207 * Determine whether the control segment from the previous postmaster 208 * invocation still exists. If so, remove the dynamic shared memory 209 * segments to which it refers, and then the control segment itself. 210 */ 211 void 212 dsm_cleanup_using_control_segment(dsm_handle old_control_handle) 213 { ...snip... 270 /* Destroy the referenced segment. */ 271 dsm_impl_op(DSM_OP_DESTROY, handle, 0, &junk_impl_private, 272 &junk_mapped_address, &junk_mapped_size, LOG); 273 } 274 275 /* Destroy the old control segment, too. */ 276 elog(DEBUG2, 277 ¦"cleaning up dynamic shared memory control segment with ID %u", 278 ¦old_control_handle); 279 dsm_impl_op(DSM_OP_DESTROY, old_control_handle, 0, &impl_private, 280 &mapped_address, &mapped_size, LOG); 281 } > > Greetings, > > Andres Freund Thanks again, Reid -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. [email protected] www.crunchydata.com ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-09 14:23 Reid Thompson <[email protected]> parent: Melanie Plageman <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Reid Thompson @ 2022-11-09 14:23 UTC (permalink / raw) To: Melanie Plageman <[email protected]>; +Cc: [email protected]; [email protected]; Drouvot, Bertrand <[email protected]> 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 <[email protected]> > > 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 +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. > > @@ -313,7 +316,9 @@ extern const char > > *pgstat_get_backend_current_activity(int pid, bool checkUser); > >  extern const char *pgstat_get_crashed_backend_activity(int pid, > > char *buffer, > >                                                                    > >                                        int buflen); > >  extern uint64 pgstat_get_my_query_id(void); > > - > > +extern void pgstat_report_backend_allocated_bytes_increase(uint64 > > allocation); > > +extern void pgstat_report_backend_allocated_bytes_decrease(uint64 > > deallocation); > > +extern uint64 pgstat_get_all_backend_memory_allocated(void); > >  > >  /* ---------- > >  * Support functions for the SQL-callable functions to > > diff --git a/src/test/regress/expected/rules.out > > b/src/test/regress/expected/rules.out > > index 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. > - Melanie > > [1] > https://www.postgresql.org/message-id/20221105024146.xxlbtsxh2niyz2fu%40awork3.anarazel.de Thanks again, Reid -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. [email protected] www.crunchydata.com ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-09 16:13 Andres Freund <[email protected]> parent: Reid Thompson <[email protected]> 1 sibling, 1 reply; 25+ messages in thread From: Andres Freund @ 2022-11-09 16:13 UTC (permalink / raw) To: Reid Thompson <[email protected]>; +Cc: [email protected]; Drouvot, Bertrand <[email protected]> Hi, 2022-11-09 08:54:54 -0500, Reid Thompson wrote: > Thanks for looking at this and for the feedback. Responses inline below. > > > +void > > > +pgstat_report_backend_allocated_bytes_decrease(uint64 > > > deallocation) > > > +{ > > > +Â Â Â Â Â Â Â volatile PgBackendStatus *beentry = MyBEEntry; > > > + > > > +Â Â Â Â Â Â Â /* > > > +Â Â Â Â Â Â Â * Cases may occur where shared memory from a previous postmaster > > > +Â Â Â Â Â Â Â * invocation still exist. These are cleaned up at startup by > > > +Â Â Â Â Â Â Â * dsm_cleanup_using_control_segment. Limit decreasing memory allocated to > > > +Â Â Â Â Â Â Â * zero in case no corresponding prior increase exists or decrease has > > > +Â Â Â Â Â Â Â * already been accounted for. > > > +Â Â Â Â Â Â Â */ > > > > I don't really follow - postmaster won't ever have a backend status > > array, so how would they be tracked here? > > On startup, a check is made for leftover dsm control segments in the > DataDir. It appears possible that in certain situations on startup we > may find and destroy stale segments and thus decrement the allocation > variable. > > I based this off of: > /ipc/dsm.c > > dsm_postmaster_startup: > 150 dsm_postmaster_startup(PGShmemHeader *shim) > { > ... > 281 } I don't think we should account for memory allocations done in postmaster in this patch. They'll otherwise be counted again in each of the forked backends. As this cleanup happens during postmaster startup, we'll have to make sure accounting is reset during backend startup. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-27 03:10 Reid Thompson <[email protected]> parent: Andres Freund <[email protected]> 0 siblings, 2 replies; 25+ messages in thread From: Reid Thompson @ 2022-11-27 03:10 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: [email protected]; [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> 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. [email protected] www.crunchydata.com Attachments: [text/x-patch] 0001-Add-tracking-of-backend-memory-allocated-to-pg_stat_.patch (32.8K, ../../[email protected]/2-0001-Add-tracking-of-backend-memory-allocated-to-pg_stat_.patch) download | inline diff: From 3d772d8620faba4bd4e091d6618c63557fbf6749 Mon Sep 17 00:00:00 2001 From: Reid Thompson <[email protected]> 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 </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>allocated_bytes</structfield> <type>bigint</type> + </para> + <para> + 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 <function>pg_size_pretty</function> + described in <xref linkend="functions-admin-dbsize"/> to make this value + more easily readable. + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>query</structfield> <type>text</type> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 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(PostmasterContext); PostmasterContext = NULL; + /* Zero allocated bytes to avoid double counting parent allocation. + * Needs to be after the MemoryContextDelete(PostmasterContext) above. + */ + pgstat_zero_my_allocated_bytes(); + AuxiliaryProcessMain(type); /* does not return */ } #endif /* EXEC_BACKEND */ @@ -5700,6 +5708,11 @@ do_start_bgworker(RegisteredBgWorker *rw) MemoryContextDelete(PostmasterContext); PostmasterContext = NULL; + /* Zero allocated bytes to avoid double counting parent allocation. + * Needs to be after the MemoryContextDelete(PostmasterContext) above. + */ + pgstat_zero_my_allocated_bytes(); + StartBackgroundWorker(); exit(1); /* should not get here */ diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c index d6d02e3c63..4cbc59cda5 100644 --- a/src/backend/postmaster/syslogger.c +++ b/src/backend/postmaster/syslogger.c @@ -679,6 +679,9 @@ SysLogger_Start(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/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c index 6ddd46a4e7..65d59fc43e 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_allocated_bytes(*mapped_size, DECREASE); *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_allocated_bytes(*mapped_size, DECREASE); + pgstat_report_allocated_bytes(request_size, INCREASE); + } +#else + pgstat_report_allocated_bytes(*mapped_size, DECREASE); + pgstat_report_allocated_bytes(request_size, INCREASE); +#endif + } *mapped_address = address; *mapped_size = request_size; close(fd); @@ -537,6 +576,14 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_allocated_bytes(*mapped_size, DECREASE); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && shmctl(ident, IPC_RMID, NULL) < 0) @@ -584,6 +631,13 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_allocated_bytes(request_size, INCREASE); *mapped_address = address; *mapped_size = request_size; @@ -652,6 +706,13 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return false; } + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_allocated_bytes(*mapped_size, DECREASE); *impl_private = NULL; *mapped_address = NULL; *mapped_size = 0; @@ -768,6 +829,12 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return false; } + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_allocated_bytes(info.RegionSize, INCREASE); *mapped_address = address; *mapped_size = info.RegionSize; *impl_private = hmap; @@ -812,6 +879,13 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + pgstat_report_allocated_bytes(*mapped_size, DECREASE); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && unlink(name) != 0) @@ -933,6 +1007,13 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_allocated_bytes(request_size, INCREASE); *mapped_address = address; *mapped_size = request_size; diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index 1146a6c33c..3785e8af53 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -49,6 +49,9 @@ int pgstat_track_activity_query_size = 1024; /* exposed so that backend_progress.c can access it */ PgBackendStatus *MyBEEntry = NULL; +/* Memory allocated to this backend prior to pgstats initialization */ +uint64 local_my_allocated_bytes = 0; +uint64 *my_allocated_bytes = &local_my_allocated_bytes; static PgBackendStatus *BackendStatusArray = NULL; static char *BackendAppnameBuffer = NULL; @@ -400,6 +403,15 @@ pgstat_bestart(void) lbeentry.st_progress_command_target = InvalidOid; lbeentry.st_query_id = UINT64CONST(0); + /* Alter allocation reporting from local_my_allocated_bytes to shared memory */ + pgstat_set_allocated_bytes_storage(&MyBEEntry->allocated_bytes); + + /* Populate sum of memory allocated prior to pgstats initialization to pgstats + * and zero the local variable. + */ + lbeentry.allocated_bytes += local_my_allocated_bytes; + local_my_allocated_bytes = 0; + /* * we don't zero st_progress_param here to save cycles; nobody should * examine it until st_progress_command has been set to something other @@ -459,6 +471,11 @@ pgstat_beshutdown_hook(int code, Datum arg) { volatile PgBackendStatus *beentry = MyBEEntry; + /* + * Stop reporting memory allocation changes to &MyBEEntry->allocated_bytes + */ + pgstat_reset_allocated_bytes_storage(); + /* * Clear my status entry, following the protocol of bumping st_changecount * before and after. We use a volatile pointer here to ensure the @@ -1191,3 +1208,31 @@ pgstat_clip_activity(const char *raw_activity) return activity; } + +/* + * Configure bytes allocated reporting to report allocated bytes to + * *allocated_bytes. *allocated_bytes needs to be valid until + * pgstat_set_allocated_bytes_storage() is called. + * + * Expected to be called during backend startup (in pgstat_bestart), to point + * my_allocated_bytes into shared memory. + */ +void +pgstat_set_allocated_bytes_storage(uint64 *new_allocated_bytes) +{ + my_allocated_bytes = new_allocated_bytes; + *new_allocated_bytes = local_my_allocated_bytes; +} + +/* + * Reset allocated bytes storage location. + * + * Expected to be called during backend shutdown, before the location set up + * by pgstat_set_allocated_bytes_storage() becomes invalid. + */ +void +pgstat_reset_allocated_bytes_storage(void) +{ + my_allocated_bytes = &local_my_allocated_bytes; +} + diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index ae3365d917..170d60df8b 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -559,7 +559,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS) Datum pg_stat_get_activity(PG_FUNCTION_ARGS) { -#define PG_STAT_GET_ACTIVITY_COLS 30 +#define PG_STAT_GET_ACTIVITY_COLS 31 int num_backends = pgstat_fetch_stat_numbackends(); int curr_backend; int pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0); @@ -615,6 +615,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) else nulls[16] = true; + values[30] = UInt64GetDatum(beentry->allocated_bytes); + /* Values only available to role member or pg_read_all_stats */ if (HAS_PGSTAT_PERMISSIONS(beentry->st_userid)) { diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index b6a8bbcd59..b202e115b6 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -47,6 +47,7 @@ #include "postgres.h" #include "port/pg_bitutils.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -521,6 +522,7 @@ AllocSetContextCreateInternal(MemoryContext parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; + pgstat_report_allocated_bytes(firstBlockSize, INCREASE); return (MemoryContext) set; } @@ -543,6 +545,7 @@ AllocSetReset(MemoryContext context) AllocSet set = (AllocSet) context; AllocBlock block; Size keepersize PG_USED_FOR_ASSERTS_ONLY; + uint64 deallocation = 0; Assert(AllocSetIsValid(set)); @@ -585,6 +588,7 @@ AllocSetReset(MemoryContext context) { /* Normal case, release the block */ context->mem_allocated -= block->endptr - ((char *) block); + deallocation += block->endptr - ((char *) block); #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -595,6 +599,7 @@ AllocSetReset(MemoryContext context) } Assert(context->mem_allocated == keepersize); + pgstat_report_allocated_bytes(deallocation, DECREASE); /* Reset block size allocation sequence, too */ set->nextBlockSize = set->initBlockSize; @@ -613,6 +618,7 @@ AllocSetDelete(MemoryContext context) AllocSet set = (AllocSet) context; AllocBlock block = set->blocks; Size keepersize PG_USED_FOR_ASSERTS_ONLY; + uint64 deallocation = 0; Assert(AllocSetIsValid(set)); @@ -651,11 +657,13 @@ AllocSetDelete(MemoryContext context) freelist->first_free = (AllocSetContext *) oldset->header.nextchild; freelist->num_free--; + deallocation += oldset->header.mem_allocated; /* All that remains is to free the header/initial block */ free(oldset); } Assert(freelist->num_free == 0); + pgstat_report_allocated_bytes(deallocation, DECREASE); } /* Now add the just-deleted context to the freelist. */ @@ -672,7 +680,10 @@ AllocSetDelete(MemoryContext context) AllocBlock next = block->next; if (block != set->keeper) + { context->mem_allocated -= block->endptr - ((char *) block); + deallocation += block->endptr - ((char *) block); + } #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -685,6 +696,7 @@ AllocSetDelete(MemoryContext context) } Assert(context->mem_allocated == keepersize); + pgstat_report_allocated_bytes(deallocation + context->mem_allocated, DECREASE); /* Finally, free the context header, including the keeper block */ free(set); @@ -734,6 +746,7 @@ AllocSetAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, INCREASE); block->aset = set; block->freeptr = block->endptr = ((char *) block) + blksize; @@ -944,6 +957,7 @@ AllocSetAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, INCREASE); block->aset = set; block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ; @@ -1043,6 +1057,7 @@ AllocSetFree(void *pointer) block->next->prev = block->prev; set->header.mem_allocated -= block->endptr - ((char *) block); + pgstat_report_allocated_bytes(block->endptr - ((char *) block), DECREASE); #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -1173,7 +1188,9 @@ AllocSetRealloc(void *pointer, Size size) /* updated separately, not to underflow when (oldblksize > blksize) */ set->header.mem_allocated -= oldblksize; + pgstat_report_allocated_bytes(oldblksize, DECREASE); set->header.mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, INCREASE); block->freeptr = block->endptr = ((char *) block) + blksize; diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c index b432a92be3..459eb985d6 100644 --- a/src/backend/utils/mmgr/generation.c +++ b/src/backend/utils/mmgr/generation.c @@ -37,6 +37,7 @@ #include "lib/ilist.h" #include "port/pg_bitutils.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -267,6 +268,7 @@ GenerationContextCreate(MemoryContext parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; + pgstat_report_allocated_bytes(firstBlockSize, INCREASE); return (MemoryContext) set; } @@ -283,6 +285,7 @@ GenerationReset(MemoryContext context) { GenerationContext *set = (GenerationContext *) context; dlist_mutable_iter miter; + uint64 deallocation = 0; Assert(GenerationIsValid(set)); @@ -305,9 +308,14 @@ GenerationReset(MemoryContext context) if (block == set->keeper) GenerationBlockMarkEmpty(block); else + { + deallocation += block->blksize; GenerationBlockFree(set, block); + } } + pgstat_report_allocated_bytes(deallocation, DECREASE); + /* set it so new allocations to make use of the keeper block */ set->block = set->keeper; @@ -328,6 +336,9 @@ GenerationDelete(MemoryContext context) { /* Reset to release all releasable GenerationBlocks */ GenerationReset(context); + + pgstat_report_allocated_bytes(context->mem_allocated, DECREASE); + /* And free the context header and keeper block */ free(context); } @@ -374,6 +385,7 @@ GenerationAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, INCREASE); /* block with a single (used) chunk */ block->context = set; @@ -477,6 +489,7 @@ GenerationAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, INCREASE); /* initialize the new block */ GenerationBlockInit(set, block, blksize); @@ -726,6 +739,8 @@ GenerationFree(void *pointer) dlist_delete(&block->node); set->header.mem_allocated -= block->blksize; + pgstat_report_allocated_bytes(block->blksize, DECREASE); + free(block); } diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c index 6df0839b6a..f38256f6f3 100644 --- a/src/backend/utils/mmgr/slab.c +++ b/src/backend/utils/mmgr/slab.c @@ -53,6 +53,7 @@ #include "postgres.h" #include "lib/ilist.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -238,6 +239,12 @@ SlabContextCreate(MemoryContext parent, parent, name); + /* + * If SlabContextCreate is updated to add headerSize to + * context->mem_allocated, then update here and SlabDelete appropriately + */ + pgstat_report_allocated_bytes(headerSize, INCREASE); + return (MemoryContext) slab; } @@ -253,6 +260,7 @@ SlabReset(MemoryContext context) { SlabContext *slab = (SlabContext *) context; int i; + uint64 deallocation = 0; Assert(SlabIsValid(slab)); @@ -278,9 +286,11 @@ SlabReset(MemoryContext context) free(block); slab->nblocks--; context->mem_allocated -= slab->blockSize; + deallocation += slab->blockSize; } } + pgstat_report_allocated_bytes(deallocation, DECREASE); slab->minFreeChunks = 0; Assert(slab->nblocks == 0); @@ -294,8 +304,17 @@ SlabReset(MemoryContext context) void SlabDelete(MemoryContext context) { + /* + * Until header allocation is included in context->mem_allocated, cast to + * slab and decrement the headerSize + */ + SlabContext *slab = castNode(SlabContext, context); + /* Reset to release all the SlabBlocks */ SlabReset(context); + + pgstat_report_allocated_bytes(slab->headerSize, DECREASE); + /* And free the context header */ free(context); } @@ -364,6 +383,7 @@ SlabAlloc(MemoryContext context, Size size) slab->minFreeChunks = slab->chunksPerBlock; slab->nblocks += 1; context->mem_allocated += slab->blockSize; + pgstat_report_allocated_bytes(slab->blockSize, INCREASE); } /* grab the block from the freelist (even the new block is there) */ @@ -537,6 +557,7 @@ SlabFree(void *pointer) free(block); slab->nblocks--; slab->header.mem_allocated -= slab->blockSize; + pgstat_report_allocated_bytes(slab->blockSize, DECREASE); } else dlist_push_head(&slab->freelist[block->nfree], &block->node); diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index f9301b2627..1bf02758d4 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5373,9 +5373,9 @@ proname => 'pg_stat_get_activity', prorows => '100', proisstrict => 'f', proretset => 't', provolatile => 's', proparallel => 'r', prorettype => 'record', proargtypes => 'int4', - proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8}', - proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id}', + proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8,int8}', + proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id,allocated_bytes}', prosrc => 'pg_stat_get_activity' }, { oid => '3318', descr => 'statistics: information about progress of backends running maintenance command', diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h index b582b46e9f..e5aa90b101 100644 --- a/src/include/utils/backend_status.h +++ b/src/include/utils/backend_status.h @@ -15,6 +15,7 @@ #include "miscadmin.h" /* for BackendType */ #include "storage/backendid.h" #include "utils/backend_progress.h" +#include "common/int.h" /* ---------- @@ -32,6 +33,13 @@ typedef enum BackendState STATE_DISABLED } BackendState; +/* Enum helper for reporting memory allocated bytes */ +enum allocation_direction +{ + DECREASE = -1, + IGNORE, + INCREASE, +}; /* ---------- * Shared-memory data structures @@ -169,6 +177,9 @@ typedef struct PgBackendStatus /* query identifier, optionally computed using post_parse_analyze_hook */ uint64 st_query_id; + + /* Current memory allocated to this backend */ + uint64 allocated_bytes; } PgBackendStatus; @@ -282,6 +293,7 @@ extern PGDLLIMPORT int pgstat_track_activity_query_size; * ---------- */ extern PGDLLIMPORT PgBackendStatus *MyBEEntry; +extern PGDLLIMPORT uint64 *my_allocated_bytes; /* ---------- @@ -313,7 +325,8 @@ extern const char *pgstat_get_backend_current_activity(int pid, bool checkUser); extern const char *pgstat_get_crashed_backend_activity(int pid, char *buffer, int buflen); extern uint64 pgstat_get_my_query_id(void); - +extern void pgstat_set_allocated_bytes_storage(uint64 *allocated_bytes); +extern void pgstat_reset_allocated_bytes_storage(void); /* ---------- * Support functions for the SQL-callable functions to @@ -325,5 +338,49 @@ extern PgBackendStatus *pgstat_fetch_stat_beentry(BackendId beid); extern LocalPgBackendStatus *pgstat_fetch_stat_local_beentry(int beid); extern char *pgstat_clip_activity(const char *raw_activity); +/* ---------- + * 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(uint64 allocated_bytes, int allocation_direction) +{ + uint64 temp; + + /* Avoid *my_allocated_bytes unsigned integer overflow on DECREASE */ + if (allocation_direction == DECREASE && + pg_sub_u64_overflow(*my_allocated_bytes, allocated_bytes, &temp)) + { + *my_allocated_bytes = 0; + ereport(LOG, + errmsg("Backend %d deallocated %ld bytes, exceeding the %ld bytes it is currently reporting allocated. Setting reported to 0.", + MyProcPid, allocated_bytes, *my_allocated_bytes)); + } + else + *my_allocated_bytes += (allocated_bytes) * allocation_direction; + + return; +} + +/* --------- + * pgstat_zero_my_allocated_bytes() - + * + * Called to zero out local allocated bytes variable after fork to avoid double + * counting allocations. + * --------- + */ +static inline void +pgstat_zero_my_allocated_bytes(void) +{ + *my_allocated_bytes = 0; + + return; +} #endif /* BACKEND_STATUS_H */ diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 37c1c86473..5263294ad4 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1756,10 +1756,11 @@ pg_stat_activity| SELECT s.datid, s.state, s.backend_xid, s.backend_xmin, + s.allocated_bytes, s.query_id, s.query, s.backend_type - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, allocated_bytes) LEFT JOIN pg_database d ON ((s.datid = d.oid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_all_indexes| SELECT c.oid AS relid, @@ -1874,7 +1875,7 @@ pg_stat_gssapi| SELECT s.pid, s.gss_auth AS gss_authenticated, s.gss_princ AS principal, s.gss_enc AS encrypted - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, allocated_bytes) WHERE (s.client_port IS NOT NULL); pg_stat_progress_analyze| SELECT s.pid, s.datid, @@ -2055,7 +2056,7 @@ pg_stat_replication| SELECT s.pid, w.sync_priority, w.sync_state, w.reply_time - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, allocated_bytes) JOIN pg_stat_get_wal_senders() w(pid, state, sent_lsn, write_lsn, flush_lsn, replay_lsn, write_lag, flush_lag, replay_lag, sync_priority, sync_state, reply_time) ON ((s.pid = w.pid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_replication_slots| SELECT s.slot_name, @@ -2089,7 +2090,7 @@ pg_stat_ssl| SELECT s.pid, s.ssl_client_dn AS client_dn, s.ssl_client_serial AS client_serial, s.ssl_issuer_dn AS issuer_dn - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, allocated_bytes) WHERE (s.client_port IS NOT NULL); pg_stat_subscription| SELECT su.oid AS subid, su.subname, diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out index 1d84407a03..ab7e95c367 100644 --- a/src/test/regress/expected/stats.out +++ b/src/test/regress/expected/stats.out @@ -1126,4 +1126,15 @@ SELECT pg_stat_get_subscription_stats(NULL); (1 row) +-- ensure that allocated_bytes exist for backends +SELECT allocated_bytes > 0 AS result FROM pg_stat_activity WHERE backend_type +IN ('checkpointer', 'background writer', 'walwriter', 'autovacuum launcher'); + result +-------- + t + t + t + t +(4 rows) + -- End of Stats Test diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql index b4d6753c71..2f0b1cc9d8 100644 --- a/src/test/regress/sql/stats.sql +++ b/src/test/regress/sql/stats.sql @@ -535,5 +535,8 @@ SET enable_seqscan TO on; SELECT pg_stat_get_replication_slot(NULL); SELECT pg_stat_get_subscription_stats(NULL); +-- ensure that allocated_bytes exist for backends +SELECT allocated_bytes > 0 AS result FROM pg_stat_activity WHERE backend_type +IN ('checkpointer', 'background writer', 'walwriter', 'autovacuum launcher'); -- End of Stats Test -- 2.25.1 ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-27 03:13 Reid Thompson <[email protected]> parent: Reid Thompson <[email protected]> 0 siblings, 0 replies; 25+ messages in thread From: Reid Thompson @ 2022-11-27 03:13 UTC (permalink / raw) To: Melanie Plageman <[email protected]>; pgsql-hackers <[email protected]>; +Cc: [email protected] 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.c... -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. [email protected] www.crunchydata.com ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-27 03:13 Reid Thompson <[email protected]> parent: Reid Thompson <[email protected]> 1 sibling, 0 replies; 25+ messages in thread From: Reid Thompson @ 2022-11-27 03:13 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: [email protected]; [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> 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.c... -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. [email protected] www.crunchydata.com ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-27 05:32 Reid Thompson <[email protected]> parent: Reid Thompson <[email protected]> 1 sibling, 3 replies; 25+ messages in thread From: Reid Thompson @ 2022-11-27 05:32 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: [email protected]; [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> 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. [email protected] www.crunchydata.com -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. [email protected] www.crunchydata.com Attachments: [text/x-patch] 0001-Add-tracking-of-backend-memory-allocated-to-pg_stat_.patch (32.8K, ../../[email protected]/2-0001-Add-tracking-of-backend-memory-allocated-to-pg_stat_.patch) download | inline diff: From fb43761181925a87cb0674b6744b009fea614796 Mon Sep 17 00:00:00 2001 From: Reid Thompson <[email protected]> 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 </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>allocated_bytes</structfield> <type>bigint</type> + </para> + <para> + 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 <function>pg_size_pretty</function> + described in <xref linkend="functions-admin-dbsize"/> to make this value + more easily readable. + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>query</structfield> <type>text</type> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 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(PostmasterContext); PostmasterContext = NULL; + /* Zero allocated bytes to avoid double counting parent allocation. + * Needs to be after the MemoryContextDelete(PostmasterContext) above. + */ + pgstat_zero_my_allocated_bytes(); + AuxiliaryProcessMain(type); /* does not return */ } #endif /* EXEC_BACKEND */ @@ -5700,6 +5708,11 @@ do_start_bgworker(RegisteredBgWorker *rw) MemoryContextDelete(PostmasterContext); PostmasterContext = NULL; + /* Zero allocated bytes to avoid double counting parent allocation. + * Needs to be after the MemoryContextDelete(PostmasterContext) above. + */ + pgstat_zero_my_allocated_bytes(); + StartBackgroundWorker(); exit(1); /* should not get here */ diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c index d6d02e3c63..4cbc59cda5 100644 --- a/src/backend/postmaster/syslogger.c +++ b/src/backend/postmaster/syslogger.c @@ -679,6 +679,9 @@ SysLogger_Start(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/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c index 6ddd46a4e7..65d59fc43e 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_allocated_bytes(*mapped_size, DECREASE); *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_allocated_bytes(*mapped_size, DECREASE); + pgstat_report_allocated_bytes(request_size, INCREASE); + } +#else + pgstat_report_allocated_bytes(*mapped_size, DECREASE); + pgstat_report_allocated_bytes(request_size, INCREASE); +#endif + } *mapped_address = address; *mapped_size = request_size; close(fd); @@ -537,6 +576,14 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_allocated_bytes(*mapped_size, DECREASE); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && shmctl(ident, IPC_RMID, NULL) < 0) @@ -584,6 +631,13 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_allocated_bytes(request_size, INCREASE); *mapped_address = address; *mapped_size = request_size; @@ -652,6 +706,13 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return false; } + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_allocated_bytes(*mapped_size, DECREASE); *impl_private = NULL; *mapped_address = NULL; *mapped_size = 0; @@ -768,6 +829,12 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return false; } + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_allocated_bytes(info.RegionSize, INCREASE); *mapped_address = address; *mapped_size = info.RegionSize; *impl_private = hmap; @@ -812,6 +879,13 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + pgstat_report_allocated_bytes(*mapped_size, DECREASE); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && unlink(name) != 0) @@ -933,6 +1007,13 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_allocated_bytes(request_size, INCREASE); *mapped_address = address; *mapped_size = request_size; diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index 1146a6c33c..3785e8af53 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -49,6 +49,9 @@ int pgstat_track_activity_query_size = 1024; /* exposed so that backend_progress.c can access it */ PgBackendStatus *MyBEEntry = NULL; +/* Memory allocated to this backend prior to pgstats initialization */ +uint64 local_my_allocated_bytes = 0; +uint64 *my_allocated_bytes = &local_my_allocated_bytes; static PgBackendStatus *BackendStatusArray = NULL; static char *BackendAppnameBuffer = NULL; @@ -400,6 +403,15 @@ pgstat_bestart(void) lbeentry.st_progress_command_target = InvalidOid; lbeentry.st_query_id = UINT64CONST(0); + /* Alter allocation reporting from local_my_allocated_bytes to shared memory */ + pgstat_set_allocated_bytes_storage(&MyBEEntry->allocated_bytes); + + /* Populate sum of memory allocated prior to pgstats initialization to pgstats + * and zero the local variable. + */ + lbeentry.allocated_bytes += local_my_allocated_bytes; + local_my_allocated_bytes = 0; + /* * we don't zero st_progress_param here to save cycles; nobody should * examine it until st_progress_command has been set to something other @@ -459,6 +471,11 @@ pgstat_beshutdown_hook(int code, Datum arg) { volatile PgBackendStatus *beentry = MyBEEntry; + /* + * Stop reporting memory allocation changes to &MyBEEntry->allocated_bytes + */ + pgstat_reset_allocated_bytes_storage(); + /* * Clear my status entry, following the protocol of bumping st_changecount * before and after. We use a volatile pointer here to ensure the @@ -1191,3 +1208,31 @@ pgstat_clip_activity(const char *raw_activity) return activity; } + +/* + * Configure bytes allocated reporting to report allocated bytes to + * *allocated_bytes. *allocated_bytes needs to be valid until + * pgstat_set_allocated_bytes_storage() is called. + * + * Expected to be called during backend startup (in pgstat_bestart), to point + * my_allocated_bytes into shared memory. + */ +void +pgstat_set_allocated_bytes_storage(uint64 *new_allocated_bytes) +{ + my_allocated_bytes = new_allocated_bytes; + *new_allocated_bytes = local_my_allocated_bytes; +} + +/* + * Reset allocated bytes storage location. + * + * Expected to be called during backend shutdown, before the location set up + * by pgstat_set_allocated_bytes_storage() becomes invalid. + */ +void +pgstat_reset_allocated_bytes_storage(void) +{ + my_allocated_bytes = &local_my_allocated_bytes; +} + diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index ae3365d917..170d60df8b 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -559,7 +559,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS) Datum pg_stat_get_activity(PG_FUNCTION_ARGS) { -#define PG_STAT_GET_ACTIVITY_COLS 30 +#define PG_STAT_GET_ACTIVITY_COLS 31 int num_backends = pgstat_fetch_stat_numbackends(); int curr_backend; int pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0); @@ -615,6 +615,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) else nulls[16] = true; + values[30] = UInt64GetDatum(beentry->allocated_bytes); + /* Values only available to role member or pg_read_all_stats */ if (HAS_PGSTAT_PERMISSIONS(beentry->st_userid)) { diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index b6a8bbcd59..b202e115b6 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -47,6 +47,7 @@ #include "postgres.h" #include "port/pg_bitutils.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -521,6 +522,7 @@ AllocSetContextCreateInternal(MemoryContext parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; + pgstat_report_allocated_bytes(firstBlockSize, INCREASE); return (MemoryContext) set; } @@ -543,6 +545,7 @@ AllocSetReset(MemoryContext context) AllocSet set = (AllocSet) context; AllocBlock block; Size keepersize PG_USED_FOR_ASSERTS_ONLY; + uint64 deallocation = 0; Assert(AllocSetIsValid(set)); @@ -585,6 +588,7 @@ AllocSetReset(MemoryContext context) { /* Normal case, release the block */ context->mem_allocated -= block->endptr - ((char *) block); + deallocation += block->endptr - ((char *) block); #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -595,6 +599,7 @@ AllocSetReset(MemoryContext context) } Assert(context->mem_allocated == keepersize); + pgstat_report_allocated_bytes(deallocation, DECREASE); /* Reset block size allocation sequence, too */ set->nextBlockSize = set->initBlockSize; @@ -613,6 +618,7 @@ AllocSetDelete(MemoryContext context) AllocSet set = (AllocSet) context; AllocBlock block = set->blocks; Size keepersize PG_USED_FOR_ASSERTS_ONLY; + uint64 deallocation = 0; Assert(AllocSetIsValid(set)); @@ -651,11 +657,13 @@ AllocSetDelete(MemoryContext context) freelist->first_free = (AllocSetContext *) oldset->header.nextchild; freelist->num_free--; + deallocation += oldset->header.mem_allocated; /* All that remains is to free the header/initial block */ free(oldset); } Assert(freelist->num_free == 0); + pgstat_report_allocated_bytes(deallocation, DECREASE); } /* Now add the just-deleted context to the freelist. */ @@ -672,7 +680,10 @@ AllocSetDelete(MemoryContext context) AllocBlock next = block->next; if (block != set->keeper) + { context->mem_allocated -= block->endptr - ((char *) block); + deallocation += block->endptr - ((char *) block); + } #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -685,6 +696,7 @@ AllocSetDelete(MemoryContext context) } Assert(context->mem_allocated == keepersize); + pgstat_report_allocated_bytes(deallocation + context->mem_allocated, DECREASE); /* Finally, free the context header, including the keeper block */ free(set); @@ -734,6 +746,7 @@ AllocSetAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, INCREASE); block->aset = set; block->freeptr = block->endptr = ((char *) block) + blksize; @@ -944,6 +957,7 @@ AllocSetAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, INCREASE); block->aset = set; block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ; @@ -1043,6 +1057,7 @@ AllocSetFree(void *pointer) block->next->prev = block->prev; set->header.mem_allocated -= block->endptr - ((char *) block); + pgstat_report_allocated_bytes(block->endptr - ((char *) block), DECREASE); #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -1173,7 +1188,9 @@ AllocSetRealloc(void *pointer, Size size) /* updated separately, not to underflow when (oldblksize > blksize) */ set->header.mem_allocated -= oldblksize; + pgstat_report_allocated_bytes(oldblksize, DECREASE); set->header.mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, INCREASE); block->freeptr = block->endptr = ((char *) block) + blksize; diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c index b432a92be3..459eb985d6 100644 --- a/src/backend/utils/mmgr/generation.c +++ b/src/backend/utils/mmgr/generation.c @@ -37,6 +37,7 @@ #include "lib/ilist.h" #include "port/pg_bitutils.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -267,6 +268,7 @@ GenerationContextCreate(MemoryContext parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; + pgstat_report_allocated_bytes(firstBlockSize, INCREASE); return (MemoryContext) set; } @@ -283,6 +285,7 @@ GenerationReset(MemoryContext context) { GenerationContext *set = (GenerationContext *) context; dlist_mutable_iter miter; + uint64 deallocation = 0; Assert(GenerationIsValid(set)); @@ -305,9 +308,14 @@ GenerationReset(MemoryContext context) if (block == set->keeper) GenerationBlockMarkEmpty(block); else + { + deallocation += block->blksize; GenerationBlockFree(set, block); + } } + pgstat_report_allocated_bytes(deallocation, DECREASE); + /* set it so new allocations to make use of the keeper block */ set->block = set->keeper; @@ -328,6 +336,9 @@ GenerationDelete(MemoryContext context) { /* Reset to release all releasable GenerationBlocks */ GenerationReset(context); + + pgstat_report_allocated_bytes(context->mem_allocated, DECREASE); + /* And free the context header and keeper block */ free(context); } @@ -374,6 +385,7 @@ GenerationAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, INCREASE); /* block with a single (used) chunk */ block->context = set; @@ -477,6 +489,7 @@ GenerationAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, INCREASE); /* initialize the new block */ GenerationBlockInit(set, block, blksize); @@ -726,6 +739,8 @@ GenerationFree(void *pointer) dlist_delete(&block->node); set->header.mem_allocated -= block->blksize; + pgstat_report_allocated_bytes(block->blksize, DECREASE); + free(block); } diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c index 6df0839b6a..f38256f6f3 100644 --- a/src/backend/utils/mmgr/slab.c +++ b/src/backend/utils/mmgr/slab.c @@ -53,6 +53,7 @@ #include "postgres.h" #include "lib/ilist.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -238,6 +239,12 @@ SlabContextCreate(MemoryContext parent, parent, name); + /* + * If SlabContextCreate is updated to add headerSize to + * context->mem_allocated, then update here and SlabDelete appropriately + */ + pgstat_report_allocated_bytes(headerSize, INCREASE); + return (MemoryContext) slab; } @@ -253,6 +260,7 @@ SlabReset(MemoryContext context) { SlabContext *slab = (SlabContext *) context; int i; + uint64 deallocation = 0; Assert(SlabIsValid(slab)); @@ -278,9 +286,11 @@ SlabReset(MemoryContext context) free(block); slab->nblocks--; context->mem_allocated -= slab->blockSize; + deallocation += slab->blockSize; } } + pgstat_report_allocated_bytes(deallocation, DECREASE); slab->minFreeChunks = 0; Assert(slab->nblocks == 0); @@ -294,8 +304,17 @@ SlabReset(MemoryContext context) void SlabDelete(MemoryContext context) { + /* + * Until header allocation is included in context->mem_allocated, cast to + * slab and decrement the headerSize + */ + SlabContext *slab = castNode(SlabContext, context); + /* Reset to release all the SlabBlocks */ SlabReset(context); + + pgstat_report_allocated_bytes(slab->headerSize, DECREASE); + /* And free the context header */ free(context); } @@ -364,6 +383,7 @@ SlabAlloc(MemoryContext context, Size size) slab->minFreeChunks = slab->chunksPerBlock; slab->nblocks += 1; context->mem_allocated += slab->blockSize; + pgstat_report_allocated_bytes(slab->blockSize, INCREASE); } /* grab the block from the freelist (even the new block is there) */ @@ -537,6 +557,7 @@ SlabFree(void *pointer) free(block); slab->nblocks--; slab->header.mem_allocated -= slab->blockSize; + pgstat_report_allocated_bytes(slab->blockSize, DECREASE); } else dlist_push_head(&slab->freelist[block->nfree], &block->node); diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index f9301b2627..1bf02758d4 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5373,9 +5373,9 @@ proname => 'pg_stat_get_activity', prorows => '100', proisstrict => 'f', proretset => 't', provolatile => 's', proparallel => 'r', prorettype => 'record', proargtypes => 'int4', - proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8}', - proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id}', + proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8,int8}', + proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id,allocated_bytes}', prosrc => 'pg_stat_get_activity' }, { oid => '3318', descr => 'statistics: information about progress of backends running maintenance command', diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h index b582b46e9f..9c9ce2e045 100644 --- a/src/include/utils/backend_status.h +++ b/src/include/utils/backend_status.h @@ -15,6 +15,7 @@ #include "miscadmin.h" /* for BackendType */ #include "storage/backendid.h" #include "utils/backend_progress.h" +#include "common/int.h" /* ---------- @@ -32,6 +33,12 @@ typedef enum BackendState STATE_DISABLED } BackendState; +/* Enum helper for reporting memory allocated bytes */ +enum allocation_direction +{ + DECREASE = -1, + INCREASE = 1, +}; /* ---------- * Shared-memory data structures @@ -169,6 +176,9 @@ typedef struct PgBackendStatus /* query identifier, optionally computed using post_parse_analyze_hook */ uint64 st_query_id; + + /* Current memory allocated to this backend */ + uint64 allocated_bytes; } PgBackendStatus; @@ -282,6 +292,7 @@ extern PGDLLIMPORT int pgstat_track_activity_query_size; * ---------- */ extern PGDLLIMPORT PgBackendStatus *MyBEEntry; +extern PGDLLIMPORT uint64 *my_allocated_bytes; /* ---------- @@ -313,7 +324,8 @@ extern const char *pgstat_get_backend_current_activity(int pid, bool checkUser); extern const char *pgstat_get_crashed_backend_activity(int pid, char *buffer, int buflen); extern uint64 pgstat_get_my_query_id(void); - +extern void pgstat_set_allocated_bytes_storage(uint64 *allocated_bytes); +extern void pgstat_reset_allocated_bytes_storage(void); /* ---------- * Support functions for the SQL-callable functions to @@ -325,5 +337,49 @@ extern PgBackendStatus *pgstat_fetch_stat_beentry(BackendId beid); extern LocalPgBackendStatus *pgstat_fetch_stat_local_beentry(int beid); extern char *pgstat_clip_activity(const char *raw_activity); +/* ---------- + * 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(uint64 allocated_bytes, int allocation_direction) +{ + uint64 temp; + + /* Avoid *my_allocated_bytes unsigned integer overflow on DECREASE */ + if (allocation_direction == DECREASE && + pg_sub_u64_overflow(*my_allocated_bytes, allocated_bytes, &temp)) + { + *my_allocated_bytes = 0; + ereport(LOG, + errmsg("Backend %d deallocated %lu bytes, exceeding the %lu bytes it is currently reporting allocated. Setting reported to 0.", + MyProcPid, allocated_bytes, *my_allocated_bytes)); + } + else + *my_allocated_bytes += (allocated_bytes) * allocation_direction; + + return; +} + +/* --------- + * pgstat_zero_my_allocated_bytes() - + * + * Called to zero out local allocated bytes variable after fork to avoid double + * counting allocations. + * --------- + */ +static inline void +pgstat_zero_my_allocated_bytes(void) +{ + *my_allocated_bytes = 0; + + return; +} #endif /* BACKEND_STATUS_H */ diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 37c1c86473..5263294ad4 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1756,10 +1756,11 @@ pg_stat_activity| SELECT s.datid, s.state, s.backend_xid, s.backend_xmin, + s.allocated_bytes, s.query_id, s.query, s.backend_type - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, allocated_bytes) LEFT JOIN pg_database d ON ((s.datid = d.oid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_all_indexes| SELECT c.oid AS relid, @@ -1874,7 +1875,7 @@ pg_stat_gssapi| SELECT s.pid, s.gss_auth AS gss_authenticated, s.gss_princ AS principal, s.gss_enc AS encrypted - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, allocated_bytes) WHERE (s.client_port IS NOT NULL); pg_stat_progress_analyze| SELECT s.pid, s.datid, @@ -2055,7 +2056,7 @@ pg_stat_replication| SELECT s.pid, w.sync_priority, w.sync_state, w.reply_time - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, allocated_bytes) JOIN pg_stat_get_wal_senders() w(pid, state, sent_lsn, write_lsn, flush_lsn, replay_lsn, write_lag, flush_lag, replay_lag, sync_priority, sync_state, reply_time) ON ((s.pid = w.pid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_replication_slots| SELECT s.slot_name, @@ -2089,7 +2090,7 @@ pg_stat_ssl| SELECT s.pid, s.ssl_client_dn AS client_dn, s.ssl_client_serial AS client_serial, s.ssl_issuer_dn AS issuer_dn - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, allocated_bytes) WHERE (s.client_port IS NOT NULL); pg_stat_subscription| SELECT su.oid AS subid, su.subname, diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out index 1d84407a03..ab7e95c367 100644 --- a/src/test/regress/expected/stats.out +++ b/src/test/regress/expected/stats.out @@ -1126,4 +1126,15 @@ SELECT pg_stat_get_subscription_stats(NULL); (1 row) +-- ensure that allocated_bytes exist for backends +SELECT allocated_bytes > 0 AS result FROM pg_stat_activity WHERE backend_type +IN ('checkpointer', 'background writer', 'walwriter', 'autovacuum launcher'); + result +-------- + t + t + t + t +(4 rows) + -- End of Stats Test diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql index b4d6753c71..2f0b1cc9d8 100644 --- a/src/test/regress/sql/stats.sql +++ b/src/test/regress/sql/stats.sql @@ -535,5 +535,8 @@ SET enable_seqscan TO on; SELECT pg_stat_get_replication_slot(NULL); SELECT pg_stat_get_subscription_stats(NULL); +-- ensure that allocated_bytes exist for backends +SELECT allocated_bytes > 0 AS result FROM pg_stat_activity WHERE backend_type +IN ('checkpointer', 'background writer', 'walwriter', 'autovacuum launcher'); -- End of Stats Test -- 2.25.1 ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-27 14:46 Justin Pryzby <[email protected]> parent: Reid Thompson <[email protected]> 2 siblings, 0 replies; 25+ messages in thread From: Justin Pryzby @ 2022-11-27 14:46 UTC (permalink / raw) To: Reid Thompson <[email protected]>; +Cc: Andres Freund <[email protected]>; [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> On Sun, Nov 27, 2022 at 12:32:19AM -0500, Reid Thompson wrote: > attempt to remedy cfbot windows build issues You can trigger those tests under your own/private repo by pushing a branch to github. See src/tools/ci/README I suppose the cfbot page ought to point that out. -- Justin ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-27 15:17 Ted Yu <[email protected]> parent: Reid Thompson <[email protected]> 2 siblings, 0 replies; 25+ messages in thread From: Ted Yu @ 2022-11-27 15:17 UTC (permalink / raw) To: [email protected]; +Cc: Andres Freund <[email protected]>; [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> On Sat, Nov 26, 2022 at 9:32 PM Reid Thompson <[email protected]> wrote: > 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 > > > Hi, + if (request_size > *mapped_size) + { + pgstat_report_allocated_bytes(*mapped_size, DECREASE); + pgstat_report_allocated_bytes(request_size, INCREASE); pgstat_report_allocated_bytes is called twice for this case. Can the two calls be combined into one (with request_size - *mapped_size, INCREASE) ? Cheers ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-27 15:40 Justin Pryzby <[email protected]> parent: Reid Thompson <[email protected]> 2 siblings, 2 replies; 25+ messages in thread From: Justin Pryzby @ 2022-11-27 15:40 UTC (permalink / raw) To: Reid Thompson <[email protected]>; +Cc: Andres Freund <[email protected]>; [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> On Sun, Nov 27, 2022 at 12:32:19AM -0500, Reid Thompson wrote: > @@ -32,6 +33,12 @@ typedef enum BackendState > STATE_DISABLED > } BackendState; > > +/* Enum helper for reporting memory allocated bytes */ > +enum allocation_direction > +{ > + DECREASE = -1, > + INCREASE = 1, > +}; 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. 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. -- Justin ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-27 18:09 Ted Yu <[email protected]> parent: Justin Pryzby <[email protected]> 1 sibling, 0 replies; 25+ messages in thread From: Ted Yu @ 2022-11-27 18:09 UTC (permalink / raw) To: Justin Pryzby <[email protected]>; +Cc: Reid Thompson <[email protected]>; Andres Freund <[email protected]>; [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> On Sun, Nov 27, 2022 at 7:41 AM Justin Pryzby <[email protected]> wrote: > On Sun, Nov 27, 2022 at 12:32:19AM -0500, Reid Thompson wrote: > > @@ -32,6 +33,12 @@ typedef enum BackendState > > STATE_DISABLED > > } BackendState; > > > > +/* Enum helper for reporting memory allocated bytes */ > > +enum allocation_direction > > +{ > > + DECREASE = -1, > > + INCREASE = 1, > > +}; > > 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. > > 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. > > -- > Justin > > > I am a bit curious: why is the allocation_direction enum needed ? pgstat_report_allocated_bytes() can be given the amount (either negative or positive) to adjust directly. Cheers ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-11-28 18:59 Andres Freund <[email protected]> parent: Reid Thompson <[email protected]> 1 sibling, 1 reply; 25+ messages in thread From: Andres Freund @ 2022-11-28 18:59 UTC (permalink / raw) To: Reid Thompson <[email protected]>; +Cc: [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> 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 ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-12-02 16:09 Reid Thompson <[email protected]> parent: Andres Freund <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Reid Thompson @ 2022-12-02 16:09 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: [email protected]; [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> 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 postgres 6389 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/4 00: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/3 00: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: 240672 Reid Thompson Senior Software Engineer Crunchy Data, Inc. [email protected] www.crunchydata.com ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-12-02 17:18 Andres Freund <[email protected]> parent: Reid Thompson <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Andres Freund @ 2022-12-02 17:18 UTC (permalink / raw) To: Reid Thompson <[email protected]>; +Cc: [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> 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 ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-12-06 13:47 Reid Thompson <[email protected]> parent: Andres Freund <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Reid Thompson @ 2022-12-06 13:47 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: [email protected]; [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> 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. [email protected] www.crunchydata.com ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-12-06 17:55 Andres Freund <[email protected]> parent: Reid Thompson <[email protected]> 0 siblings, 0 replies; 25+ messages in thread From: Andres Freund @ 2022-12-06 17:55 UTC (permalink / raw) To: Reid Thompson <[email protected]>; +Cc: [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> Hi, On 2022-12-06 08:47:55 -0500, Reid Thompson wrote: > The intent was to capture and display all the memory allocated to the > backends, for admins/users/max_total_backend_memory/others to utilize. It's going to be far less accurate than that. For one, there's a lot of operating system resources, like the page table, that are going to be ignored. We're also not going to capture allocations done directly via malloc(). There's also copy-on-write effects that we're ignoring. If we just want to show an accurate picture of the current memory usage, we need to go to operating system specific interfaces. > Why should we ignore the allocations prior to backend_status.c? It seems to add complexity without a real increase in accuracy to me. But I'm not going to push harder on it than I already have. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add tracking of backend memory allocated to pg_stat_activity @ 2022-12-08 14:09 Reid Thompson <[email protected]> parent: Justin Pryzby <[email protected]> 1 sibling, 0 replies; 25+ messages in thread From: Reid Thompson @ 2022-12-08 14:09 UTC (permalink / raw) To: Justin Pryzby <[email protected]>; +Cc: [email protected]; Andres Freund <[email protected]>; [email protected]; Drouvot, Bertrand <[email protected]>; Melanie Plageman <[email protected]> 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/[email protected] -- Reid Thompson Senior Software Engineer Crunchy Data, Inc. [email protected] www.crunchydata.com Attachments: [text/x-patch] 0001-Add-tracking-of-backend-memory-allocated-to-pg_stat_.patch (33.1K, ../../[email protected]/2-0001-Add-tracking-of-backend-memory-allocated-to-pg_stat_.patch) download | inline diff: From fdb7e6d5bb653e9c5031fd058bf168bdf80a20eb Mon Sep 17 00:00:00 2001 From: Reid Thompson <[email protected]> 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 </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>allocated_bytes</structfield> <type>bigint</type> + </para> + <para> + 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 <function>pg_size_pretty</function> + described in <xref linkend="functions-admin-dbsize"/> to make this value + more easily readable. + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>query</structfield> <type>text</type> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 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 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(PostmasterContext); PostmasterContext = NULL; + /* Zero allocated bytes to avoid double counting parent allocation. + * Needs to be after the MemoryContextDelete(PostmasterContext) above. + */ + pgstat_zero_my_allocated_bytes(); + AuxiliaryProcessMain(type); /* does not return */ } #endif /* EXEC_BACKEND */ @@ -5700,6 +5708,11 @@ do_start_bgworker(RegisteredBgWorker *rw) MemoryContextDelete(PostmasterContext); PostmasterContext = NULL; + /* Zero allocated bytes to avoid double counting parent allocation. + * Needs to be after the MemoryContextDelete(PostmasterContext) above. + */ + pgstat_zero_my_allocated_bytes(); + StartBackgroundWorker(); exit(1); /* should not get here */ diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c index d6d02e3c63..4cbc59cda5 100644 --- a/src/backend/postmaster/syslogger.c +++ b/src/backend/postmaster/syslogger.c @@ -679,6 +679,9 @@ SysLogger_Start(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/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c index 6ddd46a4e7..36ef3e425e 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_allocated_bytes(*mapped_size, PG_ALLOC_DECREASE); *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_allocated_bytes(request_size - *mapped_size, + PG_ALLOC_INCREASE); + } +#else + pgstat_report_allocated_bytes(*mapped_size, PG_ALLOC_DECREASE); + pgstat_report_allocated_bytes(request_size, PG_ALLOC_INCREASE); +#endif + } *mapped_address = address; *mapped_size = request_size; close(fd); @@ -537,6 +576,14 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_allocated_bytes(*mapped_size, PG_ALLOC_DECREASE); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && shmctl(ident, IPC_RMID, NULL) < 0) @@ -584,6 +631,13 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_allocated_bytes(request_size, PG_ALLOC_INCREASE); *mapped_address = address; *mapped_size = request_size; @@ -652,6 +706,13 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return false; } + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + if (op == DSM_OP_DESTROY) + pgstat_report_allocated_bytes(*mapped_size, PG_ALLOC_DECREASE); *impl_private = NULL; *mapped_address = NULL; *mapped_size = 0; @@ -768,6 +829,12 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size, return false; } + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_allocated_bytes(info.RegionSize, PG_ALLOC_INCREASE); *mapped_address = address; *mapped_size = info.RegionSize; *impl_private = hmap; @@ -812,6 +879,13 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Detach and destroy pass through here, only decrease the memory + * shown allocated in pg_stat_activity when the creator destroys the + * allocation. + */ + pgstat_report_allocated_bytes(*mapped_size, PG_ALLOC_DECREASE); *mapped_address = NULL; *mapped_size = 0; if (op == DSM_OP_DESTROY && unlink(name) != 0) @@ -933,6 +1007,13 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, name))); return false; } + + /* + * Attach and create pass through here, only update backend memory + * allocated in pg_stat_activity for the creator process. + */ + if (op == DSM_OP_CREATE) + pgstat_report_allocated_bytes(request_size, PG_ALLOC_INCREASE); *mapped_address = address; *mapped_size = request_size; diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index 1146a6c33c..3785e8af53 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -49,6 +49,9 @@ int pgstat_track_activity_query_size = 1024; /* exposed so that backend_progress.c can access it */ PgBackendStatus *MyBEEntry = NULL; +/* Memory allocated to this backend prior to pgstats initialization */ +uint64 local_my_allocated_bytes = 0; +uint64 *my_allocated_bytes = &local_my_allocated_bytes; static PgBackendStatus *BackendStatusArray = NULL; static char *BackendAppnameBuffer = NULL; @@ -400,6 +403,15 @@ pgstat_bestart(void) lbeentry.st_progress_command_target = InvalidOid; lbeentry.st_query_id = UINT64CONST(0); + /* Alter allocation reporting from local_my_allocated_bytes to shared memory */ + pgstat_set_allocated_bytes_storage(&MyBEEntry->allocated_bytes); + + /* Populate sum of memory allocated prior to pgstats initialization to pgstats + * and zero the local variable. + */ + lbeentry.allocated_bytes += local_my_allocated_bytes; + local_my_allocated_bytes = 0; + /* * we don't zero st_progress_param here to save cycles; nobody should * examine it until st_progress_command has been set to something other @@ -459,6 +471,11 @@ pgstat_beshutdown_hook(int code, Datum arg) { volatile PgBackendStatus *beentry = MyBEEntry; + /* + * Stop reporting memory allocation changes to &MyBEEntry->allocated_bytes + */ + pgstat_reset_allocated_bytes_storage(); + /* * Clear my status entry, following the protocol of bumping st_changecount * before and after. We use a volatile pointer here to ensure the @@ -1191,3 +1208,31 @@ pgstat_clip_activity(const char *raw_activity) return activity; } + +/* + * Configure bytes allocated reporting to report allocated bytes to + * *allocated_bytes. *allocated_bytes needs to be valid until + * pgstat_set_allocated_bytes_storage() is called. + * + * Expected to be called during backend startup (in pgstat_bestart), to point + * my_allocated_bytes into shared memory. + */ +void +pgstat_set_allocated_bytes_storage(uint64 *new_allocated_bytes) +{ + my_allocated_bytes = new_allocated_bytes; + *new_allocated_bytes = local_my_allocated_bytes; +} + +/* + * Reset allocated bytes storage location. + * + * Expected to be called during backend shutdown, before the location set up + * by pgstat_set_allocated_bytes_storage() becomes invalid. + */ +void +pgstat_reset_allocated_bytes_storage(void) +{ + my_allocated_bytes = &local_my_allocated_bytes; +} + diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 25a159b5e5..c4350cfa50 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -302,7 +302,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS) Datum pg_stat_get_activity(PG_FUNCTION_ARGS) { -#define PG_STAT_GET_ACTIVITY_COLS 30 +#define PG_STAT_GET_ACTIVITY_COLS 31 int num_backends = pgstat_fetch_stat_numbackends(); int curr_backend; int pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0); @@ -358,6 +358,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) else nulls[16] = true; + values[30] = UInt64GetDatum(beentry->allocated_bytes); + /* Values only available to role member or pg_read_all_stats */ if (HAS_PGSTAT_PERMISSIONS(beentry->st_userid)) { diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index b6a8bbcd59..cc10dfd609 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -47,6 +47,7 @@ #include "postgres.h" #include "port/pg_bitutils.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -521,6 +522,7 @@ AllocSetContextCreateInternal(MemoryContext parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; + pgstat_report_allocated_bytes(firstBlockSize, PG_ALLOC_INCREASE); return (MemoryContext) set; } @@ -543,6 +545,7 @@ AllocSetReset(MemoryContext context) AllocSet set = (AllocSet) context; AllocBlock block; Size keepersize PG_USED_FOR_ASSERTS_ONLY; + uint64 deallocation = 0; Assert(AllocSetIsValid(set)); @@ -585,6 +588,7 @@ AllocSetReset(MemoryContext context) { /* Normal case, release the block */ context->mem_allocated -= block->endptr - ((char *) block); + deallocation += block->endptr - ((char *) block); #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -595,6 +599,7 @@ AllocSetReset(MemoryContext context) } Assert(context->mem_allocated == keepersize); + pgstat_report_allocated_bytes(deallocation, PG_ALLOC_DECREASE); /* Reset block size allocation sequence, too */ set->nextBlockSize = set->initBlockSize; @@ -613,6 +618,7 @@ AllocSetDelete(MemoryContext context) AllocSet set = (AllocSet) context; AllocBlock block = set->blocks; Size keepersize PG_USED_FOR_ASSERTS_ONLY; + uint64 deallocation = 0; Assert(AllocSetIsValid(set)); @@ -651,11 +657,13 @@ AllocSetDelete(MemoryContext context) freelist->first_free = (AllocSetContext *) oldset->header.nextchild; freelist->num_free--; + deallocation += oldset->header.mem_allocated; /* All that remains is to free the header/initial block */ free(oldset); } Assert(freelist->num_free == 0); + pgstat_report_allocated_bytes(deallocation, PG_ALLOC_DECREASE); } /* Now add the just-deleted context to the freelist. */ @@ -672,7 +680,10 @@ AllocSetDelete(MemoryContext context) AllocBlock next = block->next; if (block != set->keeper) + { context->mem_allocated -= block->endptr - ((char *) block); + deallocation += block->endptr - ((char *) block); + } #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -685,6 +696,7 @@ AllocSetDelete(MemoryContext context) } Assert(context->mem_allocated == keepersize); + pgstat_report_allocated_bytes(deallocation + context->mem_allocated, PG_ALLOC_DECREASE); /* Finally, free the context header, including the keeper block */ free(set); @@ -734,6 +746,7 @@ AllocSetAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, PG_ALLOC_INCREASE); block->aset = set; block->freeptr = block->endptr = ((char *) block) + blksize; @@ -944,6 +957,7 @@ AllocSetAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, PG_ALLOC_INCREASE); block->aset = set; block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ; @@ -1043,6 +1057,7 @@ AllocSetFree(void *pointer) block->next->prev = block->prev; set->header.mem_allocated -= block->endptr - ((char *) block); + pgstat_report_allocated_bytes(block->endptr - ((char *) block), PG_ALLOC_DECREASE); #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); @@ -1173,7 +1188,9 @@ AllocSetRealloc(void *pointer, Size size) /* updated separately, not to underflow when (oldblksize > blksize) */ set->header.mem_allocated -= oldblksize; + pgstat_report_allocated_bytes(oldblksize, PG_ALLOC_DECREASE); set->header.mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, PG_ALLOC_INCREASE); block->freeptr = block->endptr = ((char *) block) + blksize; diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c index b432a92be3..b0460d97c2 100644 --- a/src/backend/utils/mmgr/generation.c +++ b/src/backend/utils/mmgr/generation.c @@ -37,6 +37,7 @@ #include "lib/ilist.h" #include "port/pg_bitutils.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -267,6 +268,7 @@ GenerationContextCreate(MemoryContext parent, name); ((MemoryContext) set)->mem_allocated = firstBlockSize; + pgstat_report_allocated_bytes(firstBlockSize, PG_ALLOC_INCREASE); return (MemoryContext) set; } @@ -283,6 +285,7 @@ GenerationReset(MemoryContext context) { GenerationContext *set = (GenerationContext *) context; dlist_mutable_iter miter; + uint64 deallocation = 0; Assert(GenerationIsValid(set)); @@ -305,9 +308,14 @@ GenerationReset(MemoryContext context) if (block == set->keeper) GenerationBlockMarkEmpty(block); else + { + deallocation += block->blksize; GenerationBlockFree(set, block); + } } + pgstat_report_allocated_bytes(deallocation, PG_ALLOC_DECREASE); + /* set it so new allocations to make use of the keeper block */ set->block = set->keeper; @@ -328,6 +336,9 @@ GenerationDelete(MemoryContext context) { /* Reset to release all releasable GenerationBlocks */ GenerationReset(context); + + pgstat_report_allocated_bytes(context->mem_allocated, PG_ALLOC_DECREASE); + /* And free the context header and keeper block */ free(context); } @@ -374,6 +385,7 @@ GenerationAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, PG_ALLOC_INCREASE); /* block with a single (used) chunk */ block->context = set; @@ -477,6 +489,7 @@ GenerationAlloc(MemoryContext context, Size size) return NULL; context->mem_allocated += blksize; + pgstat_report_allocated_bytes(blksize, PG_ALLOC_INCREASE); /* initialize the new block */ GenerationBlockInit(set, block, blksize); @@ -726,6 +739,8 @@ GenerationFree(void *pointer) dlist_delete(&block->node); set->header.mem_allocated -= block->blksize; + pgstat_report_allocated_bytes(block->blksize, PG_ALLOC_DECREASE); + free(block); } diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c index 6df0839b6a..08bb013f7c 100644 --- a/src/backend/utils/mmgr/slab.c +++ b/src/backend/utils/mmgr/slab.c @@ -53,6 +53,7 @@ #include "postgres.h" #include "lib/ilist.h" +#include "utils/backend_status.h" #include "utils/memdebug.h" #include "utils/memutils.h" #include "utils/memutils_memorychunk.h" @@ -238,6 +239,12 @@ SlabContextCreate(MemoryContext parent, parent, name); + /* + * If SlabContextCreate is updated to add headerSize to + * context->mem_allocated, then update here and SlabDelete appropriately + */ + pgstat_report_allocated_bytes(headerSize, PG_ALLOC_INCREASE); + return (MemoryContext) slab; } @@ -253,6 +260,7 @@ SlabReset(MemoryContext context) { SlabContext *slab = (SlabContext *) context; int i; + uint64 deallocation = 0; Assert(SlabIsValid(slab)); @@ -278,9 +286,11 @@ SlabReset(MemoryContext context) free(block); slab->nblocks--; context->mem_allocated -= slab->blockSize; + deallocation += slab->blockSize; } } + pgstat_report_allocated_bytes(deallocation, PG_ALLOC_DECREASE); slab->minFreeChunks = 0; Assert(slab->nblocks == 0); @@ -294,8 +304,17 @@ SlabReset(MemoryContext context) void SlabDelete(MemoryContext context) { + /* + * Until header allocation is included in context->mem_allocated, cast to + * slab and decrement the headerSize + */ + SlabContext *slab = castNode(SlabContext, context); + /* Reset to release all the SlabBlocks */ SlabReset(context); + + pgstat_report_allocated_bytes(slab->headerSize, PG_ALLOC_DECREASE); + /* And free the context header */ free(context); } @@ -364,6 +383,7 @@ SlabAlloc(MemoryContext context, Size size) slab->minFreeChunks = slab->chunksPerBlock; slab->nblocks += 1; context->mem_allocated += slab->blockSize; + pgstat_report_allocated_bytes(slab->blockSize, PG_ALLOC_INCREASE); } /* grab the block from the freelist (even the new block is there) */ @@ -537,6 +557,7 @@ SlabFree(void *pointer) free(block); slab->nblocks--; slab->header.mem_allocated -= slab->blockSize; + pgstat_report_allocated_bytes(slab->blockSize, PG_ALLOC_DECREASE); } else dlist_push_head(&slab->freelist[block->nfree], &block->node); diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index f9301b2627..1bf02758d4 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5373,9 +5373,9 @@ proname => 'pg_stat_get_activity', prorows => '100', proisstrict => 'f', proretset => 't', provolatile => 's', proparallel => 'r', prorettype => 'record', proargtypes => 'int4', - proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8}', - proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id}', + proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8,int8}', + proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id,allocated_bytes}', prosrc => 'pg_stat_get_activity' }, { oid => '3318', descr => 'statistics: information about progress of backends running maintenance command', diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h index b582b46e9f..3ba479cb0d 100644 --- a/src/include/utils/backend_status.h +++ b/src/include/utils/backend_status.h @@ -15,6 +15,7 @@ #include "miscadmin.h" /* for BackendType */ #include "storage/backendid.h" #include "utils/backend_progress.h" +#include "common/int.h" /* ---------- @@ -32,6 +33,13 @@ typedef enum BackendState STATE_DISABLED } BackendState; +/* Enum helper for reporting memory allocated bytes */ +enum allocation_direction +{ + PG_ALLOC_DECREASE = -1, + PG_ALLOC_IGNORE, + PG_ALLOC_INCREASE, +}; /* ---------- * Shared-memory data structures @@ -169,6 +177,9 @@ typedef struct PgBackendStatus /* query identifier, optionally computed using post_parse_analyze_hook */ uint64 st_query_id; + + /* Current memory allocated to this backend */ + uint64 allocated_bytes; } PgBackendStatus; @@ -282,6 +293,7 @@ extern PGDLLIMPORT int pgstat_track_activity_query_size; * ---------- */ extern PGDLLIMPORT PgBackendStatus *MyBEEntry; +extern PGDLLIMPORT uint64 *my_allocated_bytes; /* ---------- @@ -313,7 +325,8 @@ extern const char *pgstat_get_backend_current_activity(int pid, bool checkUser); extern const char *pgstat_get_crashed_backend_activity(int pid, char *buffer, int buflen); extern uint64 pgstat_get_my_query_id(void); - +extern void pgstat_set_allocated_bytes_storage(uint64 *allocated_bytes); +extern void pgstat_reset_allocated_bytes_storage(void); /* ---------- * Support functions for the SQL-callable functions to @@ -325,5 +338,49 @@ extern PgBackendStatus *pgstat_fetch_stat_beentry(BackendId beid); extern LocalPgBackendStatus *pgstat_fetch_stat_local_beentry(int beid); extern char *pgstat_clip_activity(const char *raw_activity); +/* ---------- + * 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) +{ + uint64 temp; + + /* Avoid *my_allocated_bytes unsigned integer overflow on PG_ALLOC_DECREASE */ + if (allocation_direction == PG_ALLOC_DECREASE && + pg_sub_u64_overflow(*my_allocated_bytes, allocated_bytes, &temp)) + { + *my_allocated_bytes = 0; + ereport(LOG, + errmsg("Backend %d deallocated %lld bytes, exceeding the %llu bytes it is currently reporting allocated. Setting reported to 0.", + MyProcPid, (long long)allocated_bytes, (unsigned long long)*my_allocated_bytes)); + } + else + *my_allocated_bytes += (allocated_bytes) * allocation_direction; + + return; +} + +/* --------- + * pgstat_zero_my_allocated_bytes() - + * + * Called to zero out local allocated bytes variable after fork to avoid double + * counting allocations. + * --------- + */ +static inline void +pgstat_zero_my_allocated_bytes(void) +{ + *my_allocated_bytes = 0; + + return; +} #endif /* BACKEND_STATUS_H */ diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index fb9f936d43..5f854aab18 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1756,10 +1756,11 @@ pg_stat_activity| SELECT s.datid, s.state, s.backend_xid, s.backend_xmin, + s.allocated_bytes, s.query_id, s.query, s.backend_type - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, allocated_bytes) LEFT JOIN pg_database d ON ((s.datid = d.oid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_all_indexes| SELECT c.oid AS relid, @@ -1874,7 +1875,7 @@ pg_stat_gssapi| SELECT s.pid, s.gss_auth AS gss_authenticated, s.gss_princ AS principal, s.gss_enc AS encrypted - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, allocated_bytes) WHERE (s.client_port IS NOT NULL); pg_stat_progress_analyze| SELECT s.pid, s.datid, @@ -2055,7 +2056,7 @@ pg_stat_replication| SELECT s.pid, w.sync_priority, w.sync_state, w.reply_time - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, allocated_bytes) JOIN pg_stat_get_wal_senders() w(pid, state, sent_lsn, write_lsn, flush_lsn, replay_lsn, write_lag, flush_lag, replay_lag, sync_priority, sync_state, reply_time) ON ((s.pid = w.pid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_replication_slots| SELECT s.slot_name, @@ -2089,7 +2090,7 @@ pg_stat_ssl| SELECT s.pid, s.ssl_client_dn AS client_dn, s.ssl_client_serial AS client_serial, s.ssl_issuer_dn AS issuer_dn - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id) + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, allocated_bytes) WHERE (s.client_port IS NOT NULL); pg_stat_subscription| SELECT su.oid AS subid, su.subname, diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out index 1d84407a03..ab7e95c367 100644 --- a/src/test/regress/expected/stats.out +++ b/src/test/regress/expected/stats.out @@ -1126,4 +1126,15 @@ SELECT pg_stat_get_subscription_stats(NULL); (1 row) +-- ensure that allocated_bytes exist for backends +SELECT allocated_bytes > 0 AS result FROM pg_stat_activity WHERE backend_type +IN ('checkpointer', 'background writer', 'walwriter', 'autovacuum launcher'); + result +-------- + t + t + t + t +(4 rows) + -- End of Stats Test diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql index b4d6753c71..2f0b1cc9d8 100644 --- a/src/test/regress/sql/stats.sql +++ b/src/test/regress/sql/stats.sql @@ -535,5 +535,8 @@ SET enable_seqscan TO on; SELECT pg_stat_get_replication_slot(NULL); SELECT pg_stat_get_subscription_stats(NULL); +-- ensure that allocated_bytes exist for backends +SELECT allocated_bytes > 0 AS result FROM pg_stat_activity WHERE backend_type +IN ('checkpointer', 'background writer', 'walwriter', 'autovacuum launcher'); -- End of Stats Test -- 2.25.1 ^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v24 5/8] Row pattern recognition patch (executor). @ 2024-12-19 06:06 Tatsuo Ishii <[email protected]> 0 siblings, 0 replies; 25+ messages in thread From: Tatsuo Ishii @ 2024-12-19 06:06 UTC (permalink / raw) --- src/backend/executor/nodeWindowAgg.c | 1697 +++++++++++++++++++++++++- src/backend/utils/adt/windowfuncs.c | 37 +- src/include/catalog/pg_proc.dat | 6 + src/include/nodes/execnodes.h | 30 + 4 files changed, 1759 insertions(+), 11 deletions(-) diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 70a7025818..3c2e95f46e 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -33,9 +33,13 @@ */ #include "postgres.h" +#include <regex.h> +#include <sys/types.h> + #include "access/htup_details.h" #include "catalog/objectaccess.h" #include "catalog/pg_aggregate.h" +#include "catalog/pg_collation_d.h" #include "catalog/pg_proc.h" #include "executor/executor.h" #include "executor/nodeWindowAgg.h" @@ -48,6 +52,7 @@ #include "utils/acl.h" #include "utils/builtins.h" #include "utils/datum.h" +#include "utils/fmgroids.h" #include "utils/expandeddatum.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -159,6 +164,58 @@ typedef struct WindowStatePerAggData bool restart; /* need to restart this agg in this cycle? */ } WindowStatePerAggData; +/* + * Set of StringInfo. Used in RPR. + */ +typedef struct StringSet +{ + StringInfo *str_set; + Size set_size; /* current array allocation size in number of + * items */ + int set_index; /* current used size */ +} StringSet; + +/* + * Allowed subsequent PATTERN variables positions. + * Used in RPR. + * + * pos represents the pattern variable defined order in DEFINE caluase. For + * example. "DEFINE START..., UP..., DOWN ..." and "PATTERN START UP DOWN UP" + * will create: + * VariablePos[0].pos[0] = 0; START + * VariablePos[1].pos[0] = 1; UP + * VariablePos[1].pos[1] = 3; UP + * VariablePos[2].pos[0] = 2; DOWN + * + * Note that UP has two pos because UP appears in PATTERN twice. + * + * By using this strucrture, we can know which pattern variable can be followed + * by which pattern variable(s). For example, START can be followed by UP and + * DOWN since START's pos is 0, and UP's pos is 1 or 3, DOWN's pos is 2. + * DOWN can be followed by UP since UP's pos is either 1 or 3. + * + */ + +/* + * Structure used by check_rpr_navigation() and rpr_navigation_walker(). + */ +typedef struct NavigationInfo +{ + bool is_prev; /* true if PREV */ + int num_vars; /* number of var nodes */ +} NavigationInfo; + +#define NUM_ALPHABETS 26 /* we allow [a-z] variable initials */ +typedef struct VariablePos +{ + int pos[NUM_ALPHABETS]; /* postion(s) in PATTERN */ +} VariablePos; + +/* + * Regular expression compiled cache for do_pattern_match exists + */ +static regex_t *regcache = NULL; + static void initialize_windowaggregate(WindowAggState *winstate, WindowStatePerFunc perfuncstate, WindowStatePerAgg peraggstate); @@ -184,6 +241,7 @@ static void release_partition(WindowAggState *winstate); static int row_is_in_frame(WindowAggState *winstate, int64 pos, TupleTableSlot *slot); + static void update_frameheadpos(WindowAggState *winstate); static void update_frametailpos(WindowAggState *winstate); static void update_grouptailpos(WindowAggState *winstate); @@ -195,9 +253,52 @@ static Datum GetAggInitVal(Datum textInitVal, Oid transtype); static bool are_peers(WindowAggState *winstate, TupleTableSlot *slot1, TupleTableSlot *slot2); + +static int WinGetSlotInFrame(WindowObject winobj, TupleTableSlot *slot, + int relpos, int seektype, bool set_mark, + bool *isnull, bool *isout); static bool window_gettupleslot(WindowObject winobj, int64 pos, TupleTableSlot *slot); +static void attno_map(Node *node); +static bool attno_map_walker(Node *node, void *context); +static int row_is_in_reduced_frame(WindowObject winobj, int64 pos); +static bool rpr_is_defined(WindowAggState *winstate); + +static void create_reduced_frame_map(WindowAggState *winstate); +static int get_reduced_frame_map(WindowAggState *winstate, int64 pos); +static void register_reduced_frame_map(WindowAggState *winstate, int64 pos, + int val); +static void clear_reduced_frame_map(WindowAggState *winstate); +static void update_reduced_frame(WindowObject winobj, int64 pos); + +static int64 evaluate_pattern(WindowObject winobj, int64 current_pos, + char *vname, StringInfo encoded_str, bool *result); + +static bool get_slots(WindowObject winobj, int64 current_pos); + +static int search_str_set(char *pattern, StringSet * str_set, + VariablePos * variable_pos); +static char pattern_initial(WindowAggState *winstate, char *vname); +static int do_pattern_match(char *pattern, char *encoded_str); +static void do_pattern_match_finish(void); + +static StringSet * string_set_init(void); +static void string_set_add(StringSet * string_set, StringInfo str); +static StringInfo string_set_get(StringSet * string_set, int index); +static int string_set_get_size(StringSet * string_set); +static void string_set_discard(StringSet * string_set); +static VariablePos * variable_pos_init(void); +static void variable_pos_register(VariablePos * variable_pos, char initial, + int pos); +static bool variable_pos_compare(VariablePos * variable_pos, + char initial1, char initial2); +static int variable_pos_fetch(VariablePos * variable_pos, char initial, + int index); +static void variable_pos_discard(VariablePos * variable_pos); + +static void check_rpr_navigation(Node *node, bool is_prev); +static bool rpr_navigation_walker(Node *node, void *context); /* * initialize_windowaggregate @@ -774,10 +875,12 @@ eval_windowaggregates(WindowAggState *winstate) * transition function, or * - we have an EXCLUSION clause, or * - if the new frame doesn't overlap the old one + * - if RPR is enabled * * Note that we don't strictly need to restart in the last case, but if * we're going to remove all rows from the aggregation anyway, a restart * surely is faster. + * we restart aggregation too. *---------- */ numaggs_restart = 0; @@ -788,7 +891,8 @@ eval_windowaggregates(WindowAggState *winstate) (winstate->aggregatedbase != winstate->frameheadpos && !OidIsValid(peraggstate->invtransfn_oid)) || (winstate->frameOptions & FRAMEOPTION_EXCLUSION) || - winstate->aggregatedupto <= winstate->frameheadpos) + winstate->aggregatedupto <= winstate->frameheadpos || + rpr_is_defined(winstate)) { peraggstate->restart = true; numaggs_restart++; @@ -862,7 +966,22 @@ eval_windowaggregates(WindowAggState *winstate) * head, so that tuplestore can discard unnecessary rows. */ if (agg_winobj->markptr >= 0) - WinSetMarkPosition(agg_winobj, winstate->frameheadpos); + { + int64 markpos = winstate->frameheadpos; + + if (rpr_is_defined(winstate)) + { + /* + * If RPR is used, it is possible PREV wants to look at the + * previous row. So the mark pos should be frameheadpos - 1 + * unless it is below 0. + */ + markpos -= 1; + if (markpos < 0) + markpos = 0; + } + WinSetMarkPosition(agg_winobj, markpos); + } /* * Now restart the aggregates that require it. @@ -917,6 +1036,14 @@ eval_windowaggregates(WindowAggState *winstate) { winstate->aggregatedupto = winstate->frameheadpos; ExecClearTuple(agg_row_slot); + + /* + * If RPR is defined, we do not use aggregatedupto_nonrestarted. To + * avoid assertion failure below, we reset aggregatedupto_nonrestarted + * to frameheadpos. + */ + if (rpr_is_defined(winstate)) + aggregatedupto_nonrestarted = winstate->frameheadpos; } /* @@ -930,6 +1057,12 @@ eval_windowaggregates(WindowAggState *winstate) { int ret; +#ifdef RPR_DEBUG + elog(DEBUG1, "===== loop in frame starts: aggregatedupto: " INT64_FORMAT " aggregatedbase: " INT64_FORMAT, + winstate->aggregatedupto, + winstate->aggregatedbase); +#endif + /* Fetch next row if we didn't already */ if (TupIsNull(agg_row_slot)) { @@ -945,9 +1078,53 @@ eval_windowaggregates(WindowAggState *winstate) ret = row_is_in_frame(winstate, winstate->aggregatedupto, agg_row_slot); if (ret < 0) break; + if (ret == 0) goto next_tuple; + if (rpr_is_defined(winstate)) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "reduced_frame_map: %d aggregatedupto: " INT64_FORMAT " aggregatedbase: " INT64_FORMAT, + get_reduced_frame_map(winstate, + winstate->aggregatedupto), + winstate->aggregatedupto, + winstate->aggregatedbase); +#endif + + /* + * If the row status at currentpos is already decided and current + * row status is not decided yet, it means we passed the last + * reduced frame. Time to break the loop. + */ + if (get_reduced_frame_map(winstate, + winstate->currentpos) != RF_NOT_DETERMINED && + get_reduced_frame_map(winstate, + winstate->aggregatedupto) == RF_NOT_DETERMINED) + break; + + /* + * Otherwise we need to calculate the reduced frame. + */ + ret = row_is_in_reduced_frame(winstate->agg_winobj, + winstate->aggregatedupto); + if (ret == -1) /* unmatched row */ + break; + + /* + * Check if current row needs to be skipped due to no match. + */ + if (get_reduced_frame_map(winstate, + winstate->aggregatedupto) == RF_SKIPPED && + winstate->aggregatedupto == winstate->aggregatedbase) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "skip current row for aggregation"); +#endif + break; + } + } + /* Set tuple context for evaluation of aggregate arguments */ winstate->tmpcontext->ecxt_outertuple = agg_row_slot; @@ -976,6 +1153,7 @@ next_tuple: ExecClearTuple(agg_row_slot); } + /* The frame's end is not supposed to move backwards, ever */ Assert(aggregatedupto_nonrestarted <= winstate->aggregatedupto); @@ -1199,6 +1377,7 @@ begin_partition(WindowAggState *winstate) winstate->framehead_valid = false; winstate->frametail_valid = false; winstate->grouptail_valid = false; + create_reduced_frame_map(winstate); winstate->spooled_rows = 0; winstate->currentpos = 0; winstate->frameheadpos = 0; @@ -2170,6 +2349,11 @@ ExecWindowAgg(PlanState *pstate) CHECK_FOR_INTERRUPTS(); +#ifdef RPR_DEBUG + elog(DEBUG1, "ExecWindowAgg called. pos: " INT64_FORMAT, + winstate->currentpos); +#endif + if (winstate->status == WINDOWAGG_DONE) return NULL; @@ -2278,6 +2462,17 @@ ExecWindowAgg(PlanState *pstate) /* don't evaluate the window functions when we're in pass-through mode */ if (winstate->status == WINDOWAGG_RUN) { + /* + * If RPR is defined and skip mode is next row, we need to clear + * existing reduced frame info so that we newly calculate the info + * starting from current row. + */ + if (rpr_is_defined(winstate)) + { + if (winstate->rpSkipTo == ST_NEXT_ROW) + clear_reduced_frame_map(winstate); + } + /* * Evaluate true window functions */ @@ -2444,6 +2639,9 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) TupleDesc scanDesc; ListCell *l; + TargetEntry *te; + Expr *expr; + /* check for unsupported flags */ Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK))); @@ -2542,6 +2740,16 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) winstate->temp_slot_2 = ExecInitExtraTupleSlot(estate, scanDesc, &TTSOpsMinimalTuple); + winstate->prev_slot = ExecInitExtraTupleSlot(estate, scanDesc, + &TTSOpsMinimalTuple); + + winstate->next_slot = ExecInitExtraTupleSlot(estate, scanDesc, + &TTSOpsMinimalTuple); + + winstate->null_slot = ExecInitExtraTupleSlot(estate, scanDesc, + &TTSOpsMinimalTuple); + winstate->null_slot = ExecStoreAllNullTuple(winstate->null_slot); + /* * create frame head and tail slots only if needed (must create slots in * exactly the same cases that update_frameheadpos and update_frametailpos @@ -2723,6 +2931,43 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) winstate->inRangeAsc = node->inRangeAsc; winstate->inRangeNullsFirst = node->inRangeNullsFirst; + /* Set up SKIP TO type */ + winstate->rpSkipTo = node->rpSkipTo; + /* Set up row pattern recognition PATTERN clause */ + winstate->patternVariableList = node->patternVariable; + winstate->patternRegexpList = node->patternRegexp; + + /* Set up row pattern recognition DEFINE clause */ + winstate->defineInitial = node->defineInitial; + winstate->defineVariableList = NIL; + winstate->defineClauseList = NIL; + if (node->defineClause != NIL) + { + /* + * Tweak arg var of PREV/NEXT so that it refers to scan/inner slot. + */ + foreach(l, node->defineClause) + { + char *name; + ExprState *exps; + + te = lfirst(l); + name = te->resname; + expr = te->expr; + +#ifdef RPR_DEBUG + elog(DEBUG1, "defineVariable name: %s", name); +#endif + winstate->defineVariableList = + lappend(winstate->defineVariableList, + makeString(pstrdup(name))); + attno_map((Node *) expr); + exps = ExecInitExpr(expr, (PlanState *) winstate); + winstate->defineClauseList = + lappend(winstate->defineClauseList, exps); + } + } + winstate->all_first = true; winstate->partition_spooled = false; winstate->more_partitions = false; @@ -2731,6 +2976,111 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) return winstate; } +/* + * Rewrite varno of Var nodes that are the argument of PREV/NET so that they + * see scan tuple (PREV) or inner tuple (NEXT). Also we check the arguments + * of PREV/NEXT include at least 1 column reference. This is required by the + * SQL standard. + */ +static void +attno_map(Node *node) +{ + (void) expression_tree_walker(node, attno_map_walker, NULL); +} + +static bool +attno_map_walker(Node *node, void *context) +{ + FuncExpr *func; + int nargs; + bool is_prev; + + if (node == NULL) + return false; + + if (IsA(node, FuncExpr)) + { + func = (FuncExpr *) node; + + if (func->funcid == F_PREV || func->funcid == F_NEXT) + { + /* + * The SQL standard allows to have two more arguments form of + * PREV/NEXT. But currently we allow only 1 argument form. + */ + nargs = list_length(func->args); + if (list_length(func->args) != 1) + elog(ERROR, "PREV/NEXT must have 1 argument but function %d has %d args", + func->funcid, nargs); + + /* + * Check expr of PREV/NEXT aruguments and replace varno. + */ + is_prev = (func->funcid == F_PREV) ? true : false; + check_rpr_navigation(node, is_prev); + } + } + return expression_tree_walker(node, attno_map_walker, NULL); +} + +/* + * Rewrite varno of Var of RPR navigation operations (PREV/NEXT). + * If is_prev is true, we take care PREV, otherwise NEXT. + */ +static void +check_rpr_navigation(Node *node, bool is_prev) +{ + NavigationInfo context; + + context.is_prev = is_prev; + context.num_vars = 0; + (void) expression_tree_walker(node, rpr_navigation_walker, &context); + if (context.num_vars < 1) + ereport(ERROR, + errmsg("row pattern navigation operation's argument must include at least one column reference")); +} + +static bool +rpr_navigation_walker(Node *node, void *context) +{ + NavigationInfo *nav = (NavigationInfo *) context; + + if (node == NULL) + return false; + + switch (nodeTag(node)) + { + case T_Var: + { + Var *var = (Var *) node; + + nav->num_vars++; + + if (nav->is_prev) + { + /* + * Rewrite varno from OUTER_VAR to regular var no so that + * the var references scan tuple. + */ + var->varno = var->varnosyn; + } + else + var->varno = INNER_VAR; + } + break; + case T_Const: + case T_FuncExpr: + case T_OpExpr: + break; + + default: + ereport(ERROR, + errmsg("row pattern navigation operation's argument includes unsupported expression")); + } + return expression_tree_walker(node, rpr_navigation_walker, context); +} + + /* ----------------- * ExecEndWindowAgg * ----------------- @@ -2788,6 +3138,8 @@ ExecReScanWindowAgg(WindowAggState *node) ExecClearTuple(node->agg_row_slot); ExecClearTuple(node->temp_slot_1); ExecClearTuple(node->temp_slot_2); + ExecClearTuple(node->prev_slot); + ExecClearTuple(node->next_slot); if (node->framehead_slot) ExecClearTuple(node->framehead_slot); if (node->frametail_slot) @@ -3148,7 +3500,8 @@ window_gettupleslot(WindowObject winobj, int64 pos, TupleTableSlot *slot) return false; if (pos < winobj->markpos) - elog(ERROR, "cannot fetch row before WindowObject's mark position"); + elog(ERROR, "cannot fetch row: " INT64_FORMAT " before WindowObject's mark position: " INT64_FORMAT, + pos, winobj->markpos); oldcontext = MemoryContextSwitchTo(winstate->ss.ps.ps_ExprContext->ecxt_per_query_memory); @@ -3468,14 +3821,54 @@ WinGetFuncArgInFrame(WindowObject winobj, int argno, WindowAggState *winstate; ExprContext *econtext; TupleTableSlot *slot; - int64 abs_pos; - int64 mark_pos; Assert(WindowObjectIsValid(winobj)); winstate = winobj->winstate; econtext = winstate->ss.ps.ps_ExprContext; slot = winstate->temp_slot_1; + if (WinGetSlotInFrame(winobj, slot, + relpos, seektype, set_mark, + isnull, isout) == 0) + { + econtext->ecxt_outertuple = slot; + return ExecEvalExpr((ExprState *) list_nth(winobj->argstates, argno), + econtext, isnull); + } + + if (isout) + *isout = true; + *isnull = true; + return (Datum) 0; +} + +/* + * WinGetSlotInFrame + * slot: TupleTableSlot to store the result + * relpos: signed rowcount offset from the seek position + * seektype: WINDOW_SEEK_HEAD or WINDOW_SEEK_TAIL + * set_mark: If the row is found/in frame and set_mark is true, the mark is + * moved to the row as a side-effect. + * isnull: output argument, receives isnull status of result + * isout: output argument, set to indicate whether target row position + * is out of frame (can pass NULL if caller doesn't care about this) + * + * Returns 0 if we successfullt got the slot. false if out of frame. + * (also isout is set) + */ +static int +WinGetSlotInFrame(WindowObject winobj, TupleTableSlot *slot, + int relpos, int seektype, bool set_mark, + bool *isnull, bool *isout) +{ + WindowAggState *winstate; + int64 abs_pos; + int64 mark_pos; + int num_reduced_frame; + + Assert(WindowObjectIsValid(winobj)); + winstate = winobj->winstate; + switch (seektype) { case WINDOW_SEEK_CURRENT: @@ -3542,11 +3935,25 @@ WinGetFuncArgInFrame(WindowObject winobj, int argno, winstate->frameOptions); break; } + num_reduced_frame = row_is_in_reduced_frame(winobj, + winstate->frameheadpos); + if (num_reduced_frame < 0) + goto out_of_frame; + else if (num_reduced_frame > 0) + if (relpos >= num_reduced_frame) + goto out_of_frame; break; case WINDOW_SEEK_TAIL: /* rejecting relpos > 0 is easy and simplifies code below */ if (relpos > 0) goto out_of_frame; + + /* + * RPR cares about frame head pos. Need to call + * update_frameheadpos + */ + update_frameheadpos(winstate); + update_frametailpos(winstate); abs_pos = winstate->frametailpos - 1 + relpos; @@ -3613,6 +4020,14 @@ WinGetFuncArgInFrame(WindowObject winobj, int argno, mark_pos = 0; /* keep compiler quiet */ break; } + + num_reduced_frame = row_is_in_reduced_frame(winobj, + winstate->frameheadpos + relpos); + if (num_reduced_frame < 0) + goto out_of_frame; + else if (num_reduced_frame > 0) + abs_pos = winstate->frameheadpos + relpos + + num_reduced_frame - 1; break; default: elog(ERROR, "unrecognized window seek type: %d", seektype); @@ -3631,15 +4046,13 @@ WinGetFuncArgInFrame(WindowObject winobj, int argno, *isout = false; if (set_mark) WinSetMarkPosition(winobj, mark_pos); - econtext->ecxt_outertuple = slot; - return ExecEvalExpr((ExprState *) list_nth(winobj->argstates, argno), - econtext, isnull); + return 0; out_of_frame: if (isout) *isout = true; *isnull = true; - return (Datum) 0; + return -1; } /* @@ -3670,3 +4083,1269 @@ WinGetFuncArgCurrent(WindowObject winobj, int argno, bool *isnull) return ExecEvalExpr((ExprState *) list_nth(winobj->argstates, argno), econtext, isnull); } + +/* + * rpr_is_defined + * return true if Row pattern recognition is defined. + */ +static +bool +rpr_is_defined(WindowAggState *winstate) +{ + return winstate->patternVariableList != NIL; +} + +/* + * ----------------- + * row_is_in_reduced_frame + * Determine whether a row is in the current row's reduced window frame + * according to row pattern matching + * + * The row must has been already determined that it is in a full window frame + * and fetched it into slot. + * + * Returns: + * = 0, RPR is not defined. + * >0, if the row is the first in the reduced frame. Return the number of rows + * in the reduced frame. + * -1, if the row is unmatched row + * -2, if the row is in the reduced frame but needed to be skipped because of + * AFTER MATCH SKIP PAST LAST ROW + * ----------------- + */ +static +int +row_is_in_reduced_frame(WindowObject winobj, int64 pos) +{ + WindowAggState *winstate = winobj->winstate; + int state; + int rtn; + + if (!rpr_is_defined(winstate)) + { + /* + * RPR is not defined. Assume that we are always in the the reduced + * window frame. + */ + rtn = 0; +#ifdef RPR_DEBUG + elog(DEBUG1, "row_is_in_reduced_frame returns %d: pos: " INT64_FORMAT, + rtn, pos); +#endif + return rtn; + } + + state = get_reduced_frame_map(winstate, pos); + + if (state == RF_NOT_DETERMINED) + { + update_frameheadpos(winstate); + update_reduced_frame(winobj, pos); + } + + state = get_reduced_frame_map(winstate, pos); + + switch (state) + { + int64 i; + int num_reduced_rows; + + case RF_FRAME_HEAD: + num_reduced_rows = 1; + for (i = pos + 1; + get_reduced_frame_map(winstate, i) == RF_SKIPPED; i++) + num_reduced_rows++; + rtn = num_reduced_rows; + break; + + case RF_SKIPPED: + rtn = -2; + break; + + case RF_UNMATCHED: + rtn = -1; + break; + + default: + elog(ERROR, "Unrecognized state: %d at: " INT64_FORMAT, + state, pos); + break; + } + +#ifdef RPR_DEBUG + elog(DEBUG1, "row_is_in_reduced_frame returns %d: pos: " INT64_FORMAT, + rtn, pos); +#endif + return rtn; +} + +#define REDUCED_FRAME_MAP_INIT_SIZE 1024L + +/* + * create_reduced_frame_map + * Create reduced frame map + */ +static +void +create_reduced_frame_map(WindowAggState *winstate) +{ + winstate->reduced_frame_map = + MemoryContextAlloc(winstate->partcontext, + REDUCED_FRAME_MAP_INIT_SIZE); + winstate->alloc_sz = REDUCED_FRAME_MAP_INIT_SIZE; + clear_reduced_frame_map(winstate); +} + +/* + * clear_reduced_frame_map + * Clear reduced frame map + */ +static +void +clear_reduced_frame_map(WindowAggState *winstate) +{ + Assert(winstate->reduced_frame_map != NULL); + MemSet(winstate->reduced_frame_map, RF_NOT_DETERMINED, + winstate->alloc_sz); +} + +/* + * get_reduced_frame_map + * Get reduced frame map specified by pos + */ +static +int +get_reduced_frame_map(WindowAggState *winstate, int64 pos) +{ + Assert(winstate->reduced_frame_map != NULL); + Assert(pos >= 0); + + /* + * If pos is not in the reduced frame map, it means that any info + * regarding the pos has not been registered yet. So we return + * RF_NOT_DETERMINED. + */ + if (pos >= winstate->alloc_sz) + return RF_NOT_DETERMINED; + + return winstate->reduced_frame_map[pos]; +} + +/* + * register_reduced_frame_map + * Add/replace reduced frame map member at pos. + * If there's no enough space, expand the map. + */ +static +void +register_reduced_frame_map(WindowAggState *winstate, int64 pos, int val) +{ + int64 realloc_sz; + + Assert(winstate->reduced_frame_map != NULL); + + if (pos < 0) + elog(ERROR, "wrong pos: " INT64_FORMAT, pos); + + if (pos > winstate->alloc_sz - 1) + { + realloc_sz = winstate->alloc_sz * 2; + + winstate->reduced_frame_map = + repalloc(winstate->reduced_frame_map, realloc_sz); + + MemSet(winstate->reduced_frame_map + winstate->alloc_sz, + RF_NOT_DETERMINED, realloc_sz - winstate->alloc_sz); + + winstate->alloc_sz = realloc_sz; + } + + winstate->reduced_frame_map[pos] = val; +} + +/* + * update_reduced_frame + * Update reduced frame info. + */ +static +void +update_reduced_frame(WindowObject winobj, int64 pos) +{ + WindowAggState *winstate = winobj->winstate; + ListCell *lc1, + *lc2; + bool expression_result; + int num_matched_rows; + int64 original_pos; + bool anymatch; + StringInfo encoded_str; + StringInfo pattern_str = makeStringInfo(); + StringSet *str_set; + int initial_index; + VariablePos *variable_pos; + bool greedy = false; + int64 result_pos, + i; + + /* + * Set of pattern variables evaluated to true. Each character corresponds + * to pattern variable. Example: str_set[0] = "AB"; str_set[1] = "AC"; In + * this case at row 0 A and B are true, and A and C are true in row 1. + */ + + /* initialize pattern variables set */ + str_set = string_set_init(); + + /* save original pos */ + original_pos = pos; + + /* + * Check if the pattern does not include any greedy quantifier. If it does + * not, we can just apply the pattern to each row. If it succeeds, we are + * done. + */ + foreach(lc1, winstate->patternRegexpList) + { + char *quantifier = strVal(lfirst(lc1)); + + if (*quantifier == '+' || *quantifier == '*') + { + greedy = true; + break; + } + } + + /* + * Non greedy case + */ + if (!greedy) + { + num_matched_rows = 0; + + foreach(lc1, winstate->patternVariableList) + { + char *vname = strVal(lfirst(lc1)); + + encoded_str = makeStringInfo(); + +#ifdef RPR_DEBUG + elog(DEBUG1, "pos: " INT64_FORMAT " pattern vname: %s", + pos, vname); +#endif + expression_result = false; + + /* evaluate row pattern against current row */ + result_pos = evaluate_pattern(winobj, pos, vname, + encoded_str, &expression_result); + if (!expression_result || result_pos < 0) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "expression result is false or out of frame"); +#endif + register_reduced_frame_map(winstate, original_pos, + RF_UNMATCHED); + return; + } + /* move to next row */ + pos++; + num_matched_rows++; + } +#ifdef RPR_DEBUG + elog(DEBUG1, "pattern matched"); +#endif + register_reduced_frame_map(winstate, original_pos, RF_FRAME_HEAD); + + for (i = original_pos + 1; i < original_pos + num_matched_rows; i++) + { + register_reduced_frame_map(winstate, i, RF_SKIPPED); + } + return; + } + + /* + * Greedy quantifiers included. Loop over until none of pattern matches or + * encounters end of frame. + */ + for (;;) + { + result_pos = -1; + + /* + * Loop over each PATTERN variable. + */ + anymatch = false; + encoded_str = makeStringInfo(); + + forboth(lc1, winstate->patternVariableList, lc2, + winstate->patternRegexpList) + { + char *vname = strVal(lfirst(lc1)); +#ifdef RPR_DEBUG + char *quantifier = strVal(lfirst(lc2)); + + elog(DEBUG1, "pos: " INT64_FORMAT " pattern vname: %s quantifier: %s", + pos, vname, quantifier); +#endif + expression_result = false; + + /* evaluate row pattern against current row */ + result_pos = evaluate_pattern(winobj, pos, vname, + encoded_str, &expression_result); + if (expression_result) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "expression result is true"); +#endif + anymatch = true; + } + + /* + * If out of frame, we are done. + */ + if (result_pos < 0) + break; + } + + if (!anymatch) + { + /* none of patterns matched. */ + break; + } + + string_set_add(str_set, encoded_str); + +#ifdef RPR_DEBUG + elog(DEBUG1, "pos: " INT64_FORMAT " encoded_str: %s", + encoded_str->data); +#endif + + /* move to next row */ + pos++; + + if (result_pos < 0) + { + /* out of frame */ + break; + } + } + + if (string_set_get_size(str_set) == 0) + { + /* no match found in the first row */ + register_reduced_frame_map(winstate, original_pos, RF_UNMATCHED); + return; + } + +#ifdef RPR_DEBUG + elog(DEBUG2, "pos: " INT64_FORMAT " encoded_str: %s", + pos, encoded_str->data); +#endif + + /* build regular expression */ + pattern_str = makeStringInfo(); + appendStringInfoChar(pattern_str, '^'); + initial_index = 0; + + variable_pos = variable_pos_init(); + + forboth(lc1, winstate->patternVariableList, + lc2, winstate->patternRegexpList) + { + char *vname = strVal(lfirst(lc1)); + char *quantifier = strVal(lfirst(lc2)); + char initial; + + initial = pattern_initial(winstate, vname); + Assert(initial != 0); + appendStringInfoChar(pattern_str, initial); + if (quantifier[0]) + appendStringInfoChar(pattern_str, quantifier[0]); + + /* + * Register the initial at initial_index. If the initial appears more + * than once, all of it's initial_index will be recorded. This could + * happen if a pattern variable appears in the PATTERN clause more + * than once like "UP DOWN UP" "UP UP UP". + */ + variable_pos_register(variable_pos, initial, initial_index); + + initial_index++; + } + +#ifdef RPR_DEBUG + elog(DEBUG2, "pos: " INT64_FORMAT " pattern: %s", + pos, pattern_str->data); +#endif + + /* look for matching pattern variable sequence */ +#ifdef RPR_DEBUG + elog(DEBUG1, "search_str_set started"); +#endif + num_matched_rows = search_str_set(pattern_str->data, + str_set, variable_pos); +#ifdef RPR_DEBUG + elog(DEBUG1, "search_str_set returns: %d", num_matched_rows); +#endif + variable_pos_discard(variable_pos); + string_set_discard(str_set); + + /* + * We are at the first row in the reduced frame. Save the number of + * matched rows as the number of rows in the reduced frame. + */ + if (num_matched_rows <= 0) + { + /* no match */ + register_reduced_frame_map(winstate, original_pos, RF_UNMATCHED); + } + else + { + register_reduced_frame_map(winstate, original_pos, RF_FRAME_HEAD); + + for (i = original_pos + 1; i < original_pos + num_matched_rows; i++) + { + register_reduced_frame_map(winstate, i, RF_SKIPPED); + } + } + + return; +} + +/* + * search_str_set + * Perform pattern matching using "pattern" against str_set. pattern is a + * regular expression derived from PATTERN clause. Note that the regular + * expression string is prefixed by '^' and followed by initials represented + * in a same way as str_set. str_set is a set of StringInfo. Each StringInfo + * has a string comprising initials of pattern variable strings being true in + * a row. The initials are one of [a-y], parallel to the order of variable + * names in DEFINE clause. Suppose DEFINE has variables START, UP and DOWN. If + * PATTERN has START, UP+ and DOWN, then the initials in PATTERN will be 'a', + * 'b' and 'c'. The "pattern" will be "^ab+c". + * + * variable_pos is an array representing the order of pattern variable string + * initials in PATTERN clause. For example initial 'a' potion is in + * variable_pos[0].pos[0] = 0. Note that if the pattern is "START UP DOWN UP" + * (UP appears twice), then "UP" (initial is 'b') has two position 1 and + * 3. Thus variable_pos for b is variable_pos[1].pos[0] = 1 and + * variable_pos[1].pos[1] = 3. + * + * Returns the longest number of the matching rows (greedy matching) if + * quatifier '+' or '*' is included in "pattern". + */ +static +int +search_str_set(char *pattern, StringSet * str_set, VariablePos * variable_pos) +{ +#define MAX_CANDIDATE_NUM 10000 /* max pattern match candidate size */ +#define FREEZED_CHAR 'Z' /* a pattern is freezed if it ends with the + * char */ +#define DISCARD_CHAR 'z' /* a pattern is not need to keep */ + + int set_size; /* number of rows in the set */ + int resultlen; + int index; + StringSet *old_str_set, + *new_str_set; + int new_str_size; + int len; + + set_size = string_set_get_size(str_set); + new_str_set = string_set_init(); + len = 0; + resultlen = 0; + + /* + * Generate all possible pattern variable name initials as a set of + * StringInfo named "new_str_set". For example, if we have two rows + * having "ab" (row 0) and "ac" (row 1) in the input str_set, new_str_set + * will have set of StringInfo "aa", "ac", "ba" and "bc" in the end. + */ +#ifdef RPR_DEBUG + elog(DEBUG1, "pattern: %s set_size: %d", pattern, set_size); +#endif + for (index = 0; index < set_size; index++) + { + StringInfo str; /* search target row */ + char *p; + int old_set_size; + int i; + +#ifdef RPR_DEBUG + elog(DEBUG1, "index: %d", index); +#endif + if (index == 0) + { + /* copy variables in row 0 */ + str = string_set_get(str_set, index); + p = str->data; + + /* + * Loop over each new pattern variable char. + */ + while (*p) + { + StringInfo new = makeStringInfo(); + + /* add pattern variable char */ + appendStringInfoChar(new, *p); + /* add new one to string set */ + string_set_add(new_str_set, new); +#ifdef RPR_DEBUG + elog(DEBUG1, "old_str: NULL new_str: %s", new->data); +#endif + p++; /* next pattern variable */ + } + } + else /* index != 0 */ + { + old_str_set = new_str_set; + new_str_set = string_set_init(); + str = string_set_get(str_set, index); + old_set_size = string_set_get_size(old_str_set); + + /* + * Loop over each rows in the previous result set. + */ + for (i = 0; i < old_set_size; i++) + { + StringInfo new; + char last_old_char; + int old_str_len; + StringInfo old = string_set_get(old_str_set, i); + + p = old->data; + old_str_len = strlen(p); + if (old_str_len > 0) + last_old_char = p[old_str_len - 1]; + else + last_old_char = '\0'; + + /* Is this old set freezed? */ + if (last_old_char == FREEZED_CHAR) + { + /* if shorter match. we can discard it */ + if ((old_str_len - 1) < resultlen) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "discard this old set because shorter match: %s", + old->data); +#endif + continue; + } + +#ifdef RPR_DEBUG + elog(DEBUG1, "keep this old set: %s", old->data); +#endif + + /* move the old set to new_str_set */ + string_set_add(new_str_set, old); + old_str_set->str_set[i] = NULL; + continue; + } + /* Can this old set be discarded? */ + else if (last_old_char == DISCARD_CHAR) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "discard this old set: %s", old->data); +#endif + continue; + } + +#ifdef RPR_DEBUG + elog(DEBUG1, "str->data: %s", str->data); +#endif + + /* + * loop over each pattern variable initial char in the input + * set. + */ + for (p = str->data; *p; p++) + { + /* + * Optimization. Check if the row's pattern variable + * initial character position is greater than or equal to + * the old set's last pattern variable initial character + * position. For example, if the old set's last pattern + * variable initials are "ab", then the new pattern + * variable initial can be "b" or "c" but can not be "a", + * if the initials in PATTERN is something like "a b c" or + * "a b+ c+" etc. This optimization is possible when we + * only allow "+" quantifier. + */ + if (variable_pos_compare(variable_pos, last_old_char, *p)) + { + /* copy source string */ + new = makeStringInfo(); + enlargeStringInfo(new, old->len + 1); + appendStringInfoString(new, old->data); + /* add pattern variable char */ + appendStringInfoChar(new, *p); +#ifdef RPR_DEBUG + elog(DEBUG1, "old_str: %s new_str: %s", + old->data, new->data); +#endif + + /* + * Adhoc optimization. If the first letter in the + * input string is the first and second position one + * and there's no associated quatifier '+', then we + * can dicard the input because there's no chance to + * expand the string further. + * + * For example, pattern "abc" cannot match "aa". + */ +#ifdef RPR_DEBUG + elog(DEBUG1, "pattern[1]:%c pattern[2]:%c new[0]:%c new[1]:%c", + pattern[1], pattern[2], new->data[0], new->data[1]); +#endif + if (pattern[1] == new->data[0] && + pattern[1] == new->data[1] && + pattern[2] != '+' && + pattern[1] != pattern[2]) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "discard this new data: %s", + new->data); +#endif + pfree(new->data); + pfree(new); + continue; + } + + /* add new one to string set */ + string_set_add(new_str_set, new); + } + else + { + /* + * We are freezing this pattern string. Since there's + * no chance to expand the string further, we perform + * pattern matching against the string. If it does not + * match, we can discard it. + */ + len = do_pattern_match(pattern, old->data); + + if (len <= 0) + { + /* no match. we can discard it */ + continue; + } + + else if (len <= resultlen) + { + /* shorter match. we can discard it */ + continue; + } + else + { + /* match length is the longest so far */ + + int new_index; + + /* remember the longest match */ + resultlen = len; + + /* freeze the pattern string */ + new = makeStringInfo(); + enlargeStringInfo(new, old->len + 1); + appendStringInfoString(new, old->data); + /* add freezed mark */ + appendStringInfoChar(new, FREEZED_CHAR); +#ifdef RPR_DEBUG + elog(DEBUG1, "old_str: %s new_str: %s", old->data, new->data); +#endif + string_set_add(new_str_set, new); + + /* + * Search new_str_set to find out freezed entries + * that have shorter match length. Mark them as + * "discard" so that they are discarded in the + * next round. + */ + + /* new_index_size should be the one before */ + new_str_size = + string_set_get_size(new_str_set) - 1; + + /* loop over new_str_set */ + for (new_index = 0; new_index < new_str_size; + new_index++) + { + char new_last_char; + int new_str_len; + + new = string_set_get(new_str_set, new_index); + new_str_len = strlen(new->data); + if (new_str_len > 0) + { + new_last_char = + new->data[new_str_len - 1]; + if (new_last_char == FREEZED_CHAR && + (new_str_len - 1) <= len) + { + /* + * mark this set to discard in the + * next round + */ + appendStringInfoChar(new, DISCARD_CHAR); +#ifdef RPR_DEBUG + elog(DEBUG1, "add discard char: %s", new->data); +#endif + } + } + } + } + } + } + } + /* we no longer need old string set */ + string_set_discard(old_str_set); + } + } + + /* + * Perform pattern matching to find out the longest match. + */ + new_str_size = string_set_get_size(new_str_set); +#ifdef RPR_DEBUG + elog(DEBUG1, "new_str_size: %d", new_str_size); +#endif + len = 0; + resultlen = 0; + + for (index = 0; index < new_str_size; index++) + { + StringInfo s; + + s = string_set_get(new_str_set, index); + if (s == NULL) + continue; /* no data */ + +#ifdef RPR_DEBUG + elog(DEBUG1, "target string: %s", s->data); +#endif + + /* + * If the string is already freeze, we don't need to check it by + * do_pattern_match because it has been ready checked. + */ + if (s->data[s->len] == FREEZED_CHAR) + len = s->len - 1; + + /* + * If the string is scheduled to be discarded, we just disregard it. + */ + else if (s->data[s->len] == DISCARD_CHAR) + continue; + + else + len = do_pattern_match(pattern, s->data); + + if (len > resultlen) + { + /* remember the longest match */ + resultlen = len; + + /* + * If the size of result set is equal to the number of rows in the + * set, we are done because it's not possible that the number of + * matching rows exceeds the number of rows in the set. + */ + if (resultlen >= set_size) + break; + } + } + + /* we no longer need new string set */ + string_set_discard(new_str_set); + + do_pattern_match_finish(); + return resultlen; +} + +/* + * do_pattern_match + * perform pattern match using pattern against encoded_str. + * returns matching number of rows if matching is succeeded. + * Otherwise returns 0. + */ +static +int +do_pattern_match(char *pattern, char *encoded_str) +{ + static regex_t preg; + int len; + int cflags = REG_EXTENDED; + size_t nmatch = 1; + int eflags = 0; + regmatch_t pmatch[1]; + + /* + * Compile regexp if it does not exist. + */ + if (regcache == NULL) + { + if (regcomp(&preg, pattern, cflags)) + { + elog(ERROR, "failed to compile pattern: %s", pattern); + } + regcache = &preg; + } + + if (regexec(&preg, encoded_str, nmatch, pmatch, eflags)) + return 0; /* does not match */ + + len = pmatch[0].rm_eo; /* return match length */ + return len; +} + +static +void +do_pattern_match_finish(void) +{ + if (regcache != NULL) + regfree(regcache); + regcache = NULL; +} + +/* + * evaluate_pattern + * Evaluate expression associated with PATTERN variable vname. current_pos is + * relative row position in a frame (starting from 0). If vname is evaluated + * to true, initial letters associated with vname is appended to + * encode_str. result is out paramater representing the expression evaluation + * result is true of false. + *--------- + * Return values are: + * >=0: the last match absolute row position + * otherwise out of frame. + *--------- + */ +static +int64 +evaluate_pattern(WindowObject winobj, int64 current_pos, + char *vname, StringInfo encoded_str, bool *result) +{ + WindowAggState *winstate = winobj->winstate; + ExprContext *econtext = winstate->ss.ps.ps_ExprContext; + ListCell *lc1, + *lc2, + *lc3; + ExprState *pat; + Datum eval_result; + bool out_of_frame = false; + bool isnull; + TupleTableSlot *slot; + + forthree(lc1, winstate->defineVariableList, + lc2, winstate->defineClauseList, + lc3, winstate->defineInitial) + { + char initial; /* initial letter associated with vname */ + char *name = strVal(lfirst(lc1)); + + if (strcmp(vname, name)) + continue; + + initial = *(strVal(lfirst(lc3))); + + /* set expression to evaluate */ + pat = lfirst(lc2); + + /* get current, previous and next tuples */ + if (!get_slots(winobj, current_pos)) + { + out_of_frame = true; + } + else + { + /* evaluate the expression */ + eval_result = ExecEvalExpr(pat, econtext, &isnull); + if (isnull) + { + /* expression is NULL */ +#ifdef RPR_DEBUG + elog(DEBUG1, "expression for %s is NULL at row: " INT64_FORMAT, + vname, current_pos); +#endif + *result = false; + } + else + { + if (!DatumGetBool(eval_result)) + { + /* expression is false */ +#ifdef RPR_DEBUG + elog(DEBUG1, "expression for %s is false at row: " INT64_FORMAT, + vname, current_pos); +#endif + *result = false; + } + else + { + /* expression is true */ +#ifdef RPR_DEBUG + elog(DEBUG1, "expression for %s is true at row: " INT64_FORMAT, + vname, current_pos); +#endif + appendStringInfoChar(encoded_str, initial); + *result = true; + } + } + + slot = winstate->temp_slot_1; + if (slot != winstate->null_slot) + ExecClearTuple(slot); + slot = winstate->prev_slot; + if (slot != winstate->null_slot) + ExecClearTuple(slot); + slot = winstate->next_slot; + if (slot != winstate->null_slot) + ExecClearTuple(slot); + + break; + } + + if (out_of_frame) + { + *result = false; + return -1; + } + } + return current_pos; +} + +/* + * get_slots + * Get current, previous and next tuples. + * Returns false if current row is out of partition/full frame. + */ +static +bool +get_slots(WindowObject winobj, int64 current_pos) +{ + WindowAggState *winstate = winobj->winstate; + TupleTableSlot *slot; + int ret; + ExprContext *econtext; + + econtext = winstate->ss.ps.ps_ExprContext; + + /* set up current row tuple slot */ + slot = winstate->temp_slot_1; + if (!window_gettupleslot(winobj, current_pos, slot)) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "current row is out of partition at:" INT64_FORMAT, + current_pos); +#endif + return false; + } + ret = row_is_in_frame(winstate, current_pos, slot); + if (ret <= 0) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "current row is out of frame at: " INT64_FORMAT, + current_pos); +#endif + ExecClearTuple(slot); + return false; + } + econtext->ecxt_outertuple = slot; + + /* for PREV */ + if (current_pos > 0) + { + slot = winstate->prev_slot; + if (!window_gettupleslot(winobj, current_pos - 1, slot)) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "previous row is out of partition at: " INT64_FORMAT, + current_pos - 1); +#endif + econtext->ecxt_scantuple = winstate->null_slot; + } + else + { + ret = row_is_in_frame(winstate, current_pos - 1, slot); + if (ret <= 0) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "previous row is out of frame at: " INT64_FORMAT, + current_pos - 1); +#endif + ExecClearTuple(slot); + econtext->ecxt_scantuple = winstate->null_slot; + } + else + { + econtext->ecxt_scantuple = slot; + } + } + } + else + econtext->ecxt_scantuple = winstate->null_slot; + + /* for NEXT */ + slot = winstate->next_slot; + if (!window_gettupleslot(winobj, current_pos + 1, slot)) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "next row is out of partiton at: " INT64_FORMAT, + current_pos + 1); +#endif + econtext->ecxt_innertuple = winstate->null_slot; + } + else + { + ret = row_is_in_frame(winstate, current_pos + 1, slot); + if (ret <= 0) + { +#ifdef RPR_DEBUG + elog(DEBUG1, "next row is out of frame at: " INT64_FORMAT, + current_pos + 1); +#endif + ExecClearTuple(slot); + econtext->ecxt_innertuple = winstate->null_slot; + } + else + econtext->ecxt_innertuple = slot; + } + return true; +} + +/* + * pattern_initial + * Return pattern variable initial character + * matching with pattern variable name vname. + * If not found, return 0. + */ +static +char +pattern_initial(WindowAggState *winstate, char *vname) +{ + char initial; + char *name; + ListCell *lc1, + *lc2; + + forboth(lc1, winstate->defineVariableList, + lc2, winstate->defineInitial) + { + name = strVal(lfirst(lc1)); /* DEFINE variable name */ + initial = *(strVal(lfirst(lc2))); /* DEFINE variable initial */ + + + if (!strcmp(name, vname)) + return initial; /* found */ + } + return 0; +} + +/* + * string_set_init + * Create dynamic set of StringInfo. + */ +static +StringSet * string_set_init(void) +{ +/* Initial allocation size of str_set */ +#define STRING_SET_ALLOC_SIZE 1024 + + StringSet *string_set; + Size set_size; + + string_set = palloc0(sizeof(StringSet)); + string_set->set_index = 0; + set_size = STRING_SET_ALLOC_SIZE; + string_set->str_set = palloc(set_size * sizeof(StringInfo)); + string_set->set_size = set_size; + + return string_set; +} + +/* + * string_set_add + * Add StringInfo str to StringSet string_set. + */ +static +void +string_set_add(StringSet * string_set, StringInfo str) +{ + Size set_size; + + set_size = string_set->set_size; + if (string_set->set_index >= set_size) + { + set_size *= 2; + string_set->str_set = repalloc(string_set->str_set, + set_size * sizeof(StringInfo)); + string_set->set_size = set_size; + } + + string_set->str_set[string_set->set_index++] = str; + + return; +} + +/* + * string_set_get + * Returns StringInfo specified by index. + * If there's no data yet, returns NULL. + */ +static +StringInfo +string_set_get(StringSet * string_set, int index) +{ + /* no data? */ + if (index == 0 && string_set->set_index == 0) + return NULL; + + if (index < 0 || index >= string_set->set_index) + elog(ERROR, "invalid index: %d", index); + + return string_set->str_set[index]; +} + +/* + * string_set_get_size + * Returns the size of StringSet. + */ +static +int +string_set_get_size(StringSet * string_set) +{ + return string_set->set_index; +} + +/* + * string_set_discard + * Discard StringSet. + * All memory including StringSet itself is freed. + */ +static +void +string_set_discard(StringSet * string_set) +{ + int i; + + for (i = 0; i < string_set->set_index; i++) + { + StringInfo str = string_set->str_set[i]; + + if (str) + { + pfree(str->data); + pfree(str); + } + } + pfree(string_set->str_set); + pfree(string_set); +} + +/* + * variable_pos_init + * Create and initialize variable postion structure + */ +static +VariablePos * variable_pos_init(void) +{ + VariablePos *variable_pos; + + variable_pos = palloc(sizeof(VariablePos) * NUM_ALPHABETS); + MemSet(variable_pos, -1, sizeof(VariablePos) * NUM_ALPHABETS); + return variable_pos; +} + +/* + * variable_pos_register + * Register pattern variable whose initial is initial into postion index. + * pos is position of initial. + * If pos is already registered, register it at next empty slot. + */ +static +void +variable_pos_register(VariablePos * variable_pos, char initial, int pos) +{ + int index = initial - 'a'; + int slot; + int i; + + if (pos < 0 || pos > NUM_ALPHABETS) + elog(ERROR, "initial is not valid char: %c", initial); + + for (i = 0; i < NUM_ALPHABETS; i++) + { + slot = variable_pos[index].pos[i]; + if (slot < 0) + { + /* empty slot found */ + variable_pos[index].pos[i] = pos; + return; + } + } + elog(ERROR, "no empty slot for initial: %c", initial); +} + +/* + * variable_pos_compare + * Returns true if initial1 can be followed by initial2 + */ +static +bool +variable_pos_compare(VariablePos * variable_pos, char initial1, char initial2) +{ + int index1, + index2; + int pos1, + pos2; + + for (index1 = 0;; index1++) + { + pos1 = variable_pos_fetch(variable_pos, initial1, index1); + if (pos1 < 0) + break; + + for (index2 = 0;; index2++) + { + pos2 = variable_pos_fetch(variable_pos, initial2, index2); + if (pos2 < 0) + break; + if (pos1 <= pos2) + return true; + } + } + return false; +} + +/* + * variable_pos_fetch + * Fetch position of pattern variable whose initial is initial, and whose index + * is index. If no postion was registered by initial, index, returns -1. + */ +static +int +variable_pos_fetch(VariablePos * variable_pos, char initial, int index) +{ + int pos = initial - 'a'; + + if (pos < 0 || pos > NUM_ALPHABETS) + elog(ERROR, "initial is not valid char: %c", initial); + + if (index < 0 || index > NUM_ALPHABETS) + elog(ERROR, "index is not valid: %d", index); + + return variable_pos[pos].pos[index]; +} + +/* + * variable_pos_discard + * Discard VariablePos + */ +static +void +variable_pos_discard(VariablePos * variable_pos) +{ + pfree(variable_pos); +} diff --git a/src/backend/utils/adt/windowfuncs.c b/src/backend/utils/adt/windowfuncs.c index 473c61569f..92c528d38c 100644 --- a/src/backend/utils/adt/windowfuncs.c +++ b/src/backend/utils/adt/windowfuncs.c @@ -13,6 +13,9 @@ */ #include "postgres.h" +#include "catalog/pg_collation_d.h" +#include "executor/executor.h" +#include "nodes/execnodes.h" #include "nodes/parsenodes.h" #include "nodes/supportnodes.h" #include "utils/fmgrprotos.h" @@ -37,11 +40,19 @@ typedef struct int64 remainder; /* (total rows) % (bucket num) */ } ntile_context; +/* + * rpr process information. + * Used for AFTER MATCH SKIP PAST LAST ROW + */ +typedef struct SkipContext +{ + int64 pos; /* last row absolute position */ +} SkipContext; + static bool rank_up(WindowObject winobj); static Datum leadlag_common(FunctionCallInfo fcinfo, bool forward, bool withoffset, bool withdefault); - /* * utility routine for *_rank functions. */ @@ -674,7 +685,7 @@ window_last_value(PG_FUNCTION_ARGS) bool isnull; result = WinGetFuncArgInFrame(winobj, 0, - 0, WINDOW_SEEK_TAIL, true, + 0, WINDOW_SEEK_TAIL, false, &isnull, NULL); if (isnull) PG_RETURN_NULL(); @@ -714,3 +725,25 @@ window_nth_value(PG_FUNCTION_ARGS) PG_RETURN_DATUM(result); } + +/* + * prev + * Dummy function to invoke RPR's navigation operator "PREV". + * This is *not* a window function. + */ +Datum +window_prev(PG_FUNCTION_ARGS) +{ + PG_RETURN_DATUM(PG_GETARG_DATUM(0)); +} + +/* + * next + * Dummy function to invoke RPR's navigation operation "NEXT". + * This is *not* a window function. + */ +Datum +window_next(PG_FUNCTION_ARGS) +{ + PG_RETURN_DATUM(PG_GETARG_DATUM(0)); +} diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 0f22c21723..87b2c1557f 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -10651,6 +10651,12 @@ { oid => '3114', descr => 'fetch the Nth row value', proname => 'nth_value', prokind => 'w', prorettype => 'anyelement', proargtypes => 'anyelement int4', prosrc => 'window_nth_value' }, +{ oid => '8126', descr => 'previous value', + proname => 'prev', provolatile => 's', prorettype => 'anyelement', + proargtypes => 'anyelement', prosrc => 'window_prev' }, +{ oid => '8127', descr => 'next value', + proname => 'next', provolatile => 's', prorettype => 'anyelement', + proargtypes => 'anyelement', prosrc => 'window_next' }, # functions for range types { oid => '3832', descr => 'I/O', diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 7f71b7625d..a8e2404f83 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -2587,6 +2587,11 @@ typedef enum WindowAggStatus * tuples during spool */ } WindowAggStatus; +#define RF_NOT_DETERMINED 0 +#define RF_FRAME_HEAD 1 +#define RF_SKIPPED 2 +#define RF_UNMATCHED 3 + typedef struct WindowAggState { ScanState ss; /* its first field is NodeTag */ @@ -2646,6 +2651,19 @@ typedef struct WindowAggState int64 groupheadpos; /* current row's peer group head position */ int64 grouptailpos; /* " " " " tail position (group end+1) */ + /* these fields are used in Row pattern recognition: */ + RPSkipTo rpSkipTo; /* Row Pattern Skip To type */ + List *patternVariableList; /* list of row pattern variables names + * (list of String) */ + List *patternRegexpList; /* list of row pattern regular expressions + * ('+' or ''. list of String) */ + List *defineVariableList; /* list of row pattern definition + * variables (list of String) */ + List *defineClauseList; /* expression for row pattern definition + * search conditions ExprState list */ + List *defineInitial; /* list of row pattern definition variable + * initials (list of String) */ + MemoryContext partcontext; /* context for partition-lifespan data */ MemoryContext aggcontext; /* shared context for aggregate working data */ MemoryContext curaggcontext; /* current aggregate's working data */ @@ -2673,6 +2691,18 @@ typedef struct WindowAggState TupleTableSlot *agg_row_slot; TupleTableSlot *temp_slot_1; TupleTableSlot *temp_slot_2; + + /* temporary slots for RPR */ + TupleTableSlot *prev_slot; /* PREV row navigation operator */ + TupleTableSlot *next_slot; /* NEXT row navigation operator */ + TupleTableSlot *null_slot; /* all NULL slot */ + + /* + * Each byte corresponds to a row positioned at absolute its pos in + * partition. See above definition for RF_* + */ + char *reduced_frame_map; + int64 alloc_sz; /* size of the map */ } WindowAggState; /* ---------------- -- 2.25.1 ----Next_Part(Thu_Dec_19_15_19_50_2024_894)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v24-0006-Row-pattern-recognition-patch-docs.patch" ^ permalink raw reply [nested|flat] 25+ messages in thread
end of thread, other threads:[~2024-12-19 06:06 UTC | newest] Thread overview: 25+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-03 22:36 [PATCH 2/2] doc review Justin Pryzby <[email protected]> 2022-10-25 18:51 Re: Add tracking of backend memory allocated to pg_stat_activity Reid Thompson <[email protected]> 2022-10-25 18:59 ` Re: Add tracking of backend memory allocated to pg_stat_activity Reid Thompson <[email protected]> 2022-11-04 10:06 ` Re: Add tracking of backend memory allocated to pg_stat_activity Drouvot, Bertrand <[email protected]> 2022-11-04 12:56 ` Re: Add tracking of backend memory allocated to pg_stat_activity Reid Thompson <[email protected]> 2022-11-05 02:41 ` Re: Add tracking of backend memory allocated to pg_stat_activity Andres Freund <[email protected]> 2022-11-09 13:54 ` Re: Add tracking of backend memory allocated to pg_stat_activity Reid Thompson <[email protected]> 2022-11-09 16:13 ` Re: Add tracking of backend memory allocated to pg_stat_activity Andres Freund <[email protected]> 2022-11-27 03:10 ` Re: Add tracking of backend memory allocated to pg_stat_activity Reid Thompson <[email protected]> 2022-11-27 05:32 ` Re: Add tracking of backend memory allocated to pg_stat_activity Reid Thompson <[email protected]> 2022-11-27 14:46 ` Re: Add tracking of backend memory allocated to pg_stat_activity Justin Pryzby <[email protected]> 2022-11-27 15:17 ` Re: Add tracking of backend memory allocated to pg_stat_activity Ted Yu <[email protected]> 2022-11-27 15:40 ` Re: Add tracking of backend memory allocated to pg_stat_activity Justin Pryzby <[email protected]> 2022-11-27 18:09 ` Re: Add tracking of backend memory allocated to pg_stat_activity Ted Yu <[email protected]> 2022-12-08 14:09 ` Re: Add tracking of backend memory allocated to pg_stat_activity Reid Thompson <[email protected]> 2022-11-28 18:59 ` Re: Add tracking of backend memory allocated to pg_stat_activity Andres Freund <[email protected]> 2022-12-02 16:09 ` Re: Add tracking of backend memory allocated to pg_stat_activity Reid Thompson <[email protected]> 2022-12-02 17:18 ` Re: Add tracking of backend memory allocated to pg_stat_activity Andres Freund <[email protected]> 2022-12-06 13:47 ` Re: Add tracking of backend memory allocated to pg_stat_activity Reid Thompson <[email protected]> 2022-12-06 17:55 ` Re: Add tracking of backend memory allocated to pg_stat_activity Andres Freund <[email protected]> 2022-11-27 03:13 ` Re: Add tracking of backend memory allocated to pg_stat_activity Reid Thompson <[email protected]> 2022-11-07 21:17 ` Re: Add tracking of backend memory allocated to pg_stat_activity Melanie Plageman <[email protected]> 2022-11-09 14:23 ` Re: Add tracking of backend memory allocated to pg_stat_activity Reid Thompson <[email protected]> 2022-11-27 03:13 ` Re: Add tracking of backend memory allocated to pg_stat_activity Reid Thompson <[email protected]> 2024-12-19 06:06 [PATCH v24 5/8] Row pattern recognition patch (executor). Tatsuo Ishii <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox