public inbox for [email protected]
help / color / mirror / Atom feedParent/child context relation in pg_get_backend_memory_contexts()
19+ messages / 8 participants
[nested] [flat]
* Parent/child context relation in pg_get_backend_memory_contexts()
@ 2023-06-16 14:03 Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Melih Mutlu @ 2023-06-16 14:03 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Hi hackers,
pg_get_backend_memory_contexts() (and pg_backend_memory_contexts view)
does not display parent/child relation between contexts reliably.
Current version of this function only shows the name of parent context
for each context. The issue here is that it's not guaranteed that
context names are unique. So, this makes it difficult to find the
correct parent of a context.
How can knowing the correct parent context be useful? One important
use-case can be that it would allow us to sum up all the space used by
a particular context and all other subcontexts which stem from that
context.
Calculating this sum is helpful since currently
(total/used/free)_bytes returned by this function does not include
child contexts. For this reason, only looking into the related row in
pg_backend_memory_contexts does not help us to understand how many
bytes that context is actually taking.
Simplest approach to solve this could be just adding two new fields,
id and parent_id, in pg_get_backend_memory_contexts() and ensuring
each context has a unique id. This way allows us to build a correct
memory context "tree".
Please see the attached patch which introduces those two fields.
Couldn't find an existing unique identifier to use. The patch simply
assigns an id during the execution of
pg_get_backend_memory_contexts() and does not store those id's
anywhere. This means that these id's may be different in each call.
With this change, here's a query to find how much space used by each
context including its children:
> WITH RECURSIVE cte AS (
> SELECT id, total_bytes, id as root, name as root_name
> FROM memory_contexts
> UNION ALL
> SELECT r.id, r.total_bytes, cte.root, cte.root_name
> FROM memory_contexts r
> INNER JOIN cte ON r.parent_id = cte.id
> ),
> memory_contexts AS (
> SELECT * FROM pg_backend_memory_contexts
> )
> SELECT root as id, root_name as name, sum(total_bytes)
> FROM cte
> GROUP BY root, root_name
> ORDER BY sum DESC;
You should see that TopMemoryContext is the one with highest allocated
space since all other contexts are simply created under
TopMemoryContext.
Also; even though having a correct link between parent/child contexts
can be useful to find out many other things as well by only writing
SQL queries, it might require complex recursive queries similar to the
one in case of total_bytes including children. Maybe, we can also
consider adding such frequently used and/or useful information as new
fields in pg_get_backend_memory_contexts() too.
I appreciate any comment/feedback on this.
Thanks,
--
Melih Mutlu
Microsoft
Attachments:
[application/octet-stream] 0001-Adding-id-parent_id-into-pg_backend_memory_contexts.patch (5.6K, ../../CAGPVpCThLyOsj3e_gYEvLoHkr5w=tadDiN_=z2OwsK3VJppeBA@mail.gmail.com/2-0001-Adding-id-parent_id-into-pg_backend_memory_contexts.patch)
download | inline diff:
From 1699504b3773566a76f624a198d52208226fbeff Mon Sep 17 00:00:00 2001
From: Melih Mutlu <[email protected]>
Date: Tue, 13 Jun 2023 15:00:05 +0300
Subject: [PATCH] Adding id/parent_id into pg_backend_memory_contexts
Added two new fields into the tuples returned by
pg_get_backend_memory_contexts() to specify the parent/child relation
between memory contexts.
Context names cannot be relied on since they're not unique. Therefore,
unique id and parent_id are needed for each context. Those new id's are
assigned during pg_get_backend_memory_contexts() call and not stored
anywhere. So they may change in each pg_get_backend_memory_contexts()
call and shouldn't be used across differenct
pg_get_backend_memory_contexts() calls.
Context id's are start from 0 which is assigned to TopMemoryContext.
---
doc/src/sgml/system-views.sgml | 18 ++++++++++++++++++
src/backend/utils/adt/mcxtfuncs.c | 19 ++++++++++++++-----
src/include/catalog/pg_proc.dat | 6 +++---
src/test/regress/expected/rules.out | 6 ++++--
4 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 57b228076e..5d216ddf7b 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -538,6 +538,24 @@
Used space in bytes
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>context_id</structfield> <type>int4</type>
+ </para>
+ <para>
+ Current context id
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>parent_id</structfield> <type>int4</type>
+ </para>
+ <para>
+ Parent context id
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index 92ca5b2f72..bd8943cd83 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -35,9 +35,10 @@
static void
PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
TupleDesc tupdesc, MemoryContext context,
- const char *parent, int level)
+ const char *parent, int level, int *context_id,
+ int parent_id)
{
-#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 9
+#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 11
Datum values[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
bool nulls[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
@@ -45,6 +46,7 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
MemoryContext child;
const char *name;
const char *ident;
+ int current_context_id = (*context_id)++;
Assert(MemoryContextIsValid(context));
@@ -103,12 +105,18 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
values[6] = Int64GetDatum(stat.freespace);
values[7] = Int64GetDatum(stat.freechunks);
values[8] = Int64GetDatum(stat.totalspace - stat.freespace);
+ values[9] = Int32GetDatum(current_context_id);
+ if(parent_id < 0)
+ /* TopMemoryContext has no parent context */
+ nulls[10] = true;
+ else
+ values[10] = Int32GetDatum(parent_id);
tuplestore_putvalues(tupstore, tupdesc, values, nulls);
for (child = context->firstchild; child != NULL; child = child->nextchild)
{
- PutMemoryContextsStatsTupleStore(tupstore, tupdesc,
- child, name, level + 1);
+ PutMemoryContextsStatsTupleStore(tupstore, tupdesc, child,
+ name, level + 1, context_id, current_context_id);
}
}
@@ -120,10 +128,11 @@ Datum
pg_get_backend_memory_contexts(PG_FUNCTION_ARGS)
{
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+ int context_id = 0;
InitMaterializedSRF(fcinfo, 0);
PutMemoryContextsStatsTupleStore(rsinfo->setResult, rsinfo->setDesc,
- TopMemoryContext, NULL, 0);
+ TopMemoryContext, NULL, 0, &context_id, -1);
return (Datum) 0;
}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 6996073989..dcdb30da7a 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8212,9 +8212,9 @@
proname => 'pg_get_backend_memory_contexts', prorows => '100',
proretset => 't', provolatile => 'v', proparallel => 'r',
prorettype => 'record', proargtypes => '',
- proallargtypes => '{text,text,text,int4,int8,int8,int8,int8,int8}',
- proargmodes => '{o,o,o,o,o,o,o,o,o}',
- proargnames => '{name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes}',
+ proallargtypes => '{text,text,text,int4,int8,int8,int8,int8,int8,int4,int4}',
+ proargmodes => '{o,o,o,o,o,o,o,o,o,o,o}',
+ proargnames => '{name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes, id, parent_id}',
prosrc => 'pg_get_backend_memory_contexts' },
# logging memory contexts of the specified backend
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 7fd81e6a7d..ba6eab5a9c 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1311,8 +1311,10 @@ pg_backend_memory_contexts| SELECT name,
total_nblocks,
free_bytes,
free_chunks,
- used_bytes
- FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes);
+ used_bytes,
+ id,
+ parent_id
+ FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes, id, parent_id);
pg_config| SELECT name,
setting
FROM pg_config() pg_config(name, setting);
--
2.25.1
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
@ 2023-08-04 18:16 ` Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
2023-10-18 19:53 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Stephen Frost <[email protected]>
0 siblings, 2 replies; 19+ messages in thread
From: Melih Mutlu @ 2023-08-04 18:16 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Hi hackers,
Melih Mutlu <[email protected]>, 16 Haz 2023 Cum, 17:03 tarihinde şunu
yazdı:
> With this change, here's a query to find how much space used by each
> context including its children:
>
> > WITH RECURSIVE cte AS (
> > SELECT id, total_bytes, id as root, name as root_name
> > FROM memory_contexts
> > UNION ALL
> > SELECT r.id, r.total_bytes, cte.root, cte.root_name
> > FROM memory_contexts r
> > INNER JOIN cte ON r.parent_id = cte.id
> > ),
> > memory_contexts AS (
> > SELECT * FROM pg_backend_memory_contexts
> > )
> > SELECT root as id, root_name as name, sum(total_bytes)
> > FROM cte
> > GROUP BY root, root_name
> > ORDER BY sum DESC;
>
Given that the above query to get total bytes including all children is
still a complex one, I decided to add an additional info in
pg_backend_memory_contexts.
The new "path" field displays an integer array that consists of ids of all
parents for the current context. This way it's easier to tell whether a
context is a child of another context, and we don't need to use recursive
queries to get this info.
Here how pg_backend_memory_contexts would look like with this patch:
postgres=# SELECT name, id, parent, parent_id, path
FROM pg_backend_memory_contexts
ORDER BY total_bytes DESC LIMIT 10;
name | id | parent | parent_id | path
-------------------------+-----+------------------+-----------+--------------
CacheMemoryContext | 27 | TopMemoryContext | 0 | {0}
Timezones | 124 | TopMemoryContext | 0 | {0}
TopMemoryContext | 0 | | |
MessageContext | 8 | TopMemoryContext | 0 | {0}
WAL record construction | 118 | TopMemoryContext | 0 | {0}
ExecutorState | 18 | PortalContext | 17 | {0,16,17}
TupleSort main | 19 | ExecutorState | 18 | {0,16,17,18}
TransactionAbortContext | 14 | TopMemoryContext | 0 | {0}
smgr relation table | 10 | TopMemoryContext | 0 | {0}
GUC hash table | 123 | GUCMemoryContext | 122 | {0,122}
(10 rows)
An example query to calculate the total_bytes including its children for a
context (say CacheMemoryContext) would look like this:
WITH contexts AS (
SELECT * FROM pg_backend_memory_contexts
)
SELECT sum(total_bytes)
FROM contexts
WHERE ARRAY[(SELECT id FROM contexts WHERE name = 'CacheMemoryContext')] <@
path;
We still need to use cte since ids are not persisted and might change in
each run of pg_backend_memory_contexts. Materializing the result can
prevent any inconsistencies due to id change. Also it can be even good for
performance reasons as well.
Any thoughts?
Thanks,
--
Melih Mutlu
Microsoft
Attachments:
[application/octet-stream] v2-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch (7.3K, ../../CAGPVpCRNGd4V6MsxOttDZG4Hbf-4c1rrQKL4CsLyhaSY4sNkjA@mail.gmail.com/3-v2-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch)
download | inline diff:
From e9134ad5465d0dae626cf658def2ca58648d64c2 Mon Sep 17 00:00:00 2001
From: Melih Mutlu <[email protected]>
Date: Tue, 13 Jun 2023 15:00:05 +0300
Subject: [PATCH v2] Adding id/parent_id into pg_backend_memory_contexts
Added three new fields into the tuples returned by
pg_get_backend_memory_contexts() to specify the parent/child relation
between memory contexts and the path from root to current context.
Context names cannot be relied on since they're not unique. Therefore,
unique id and parent_id are needed for each context. Those new id's are
assigned during pg_get_backend_memory_contexts() call and not stored
anywhere. So they may change in each pg_get_backend_memory_contexts()
call and shouldn't be used across differenct
pg_get_backend_memory_contexts() calls.
Context id's are start from 0 which is assigned to TopMemoryContext.
---
doc/src/sgml/system-views.sgml | 27 +++++++++++++
src/backend/utils/adt/mcxtfuncs.c | 59 ++++++++++++++++++++++++++---
src/include/catalog/pg_proc.dat | 6 +--
src/test/regress/expected/rules.out | 7 +++-
4 files changed, 89 insertions(+), 10 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 57b228076e..7811130cfb 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -538,6 +538,33 @@
Used space in bytes
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>context_id</structfield> <type>int4</type>
+ </para>
+ <para>
+ Current context id
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>parent_id</structfield> <type>int4</type>
+ </para>
+ <para>
+ Parent context id
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>path</structfield> <type>int4</type>
+ </para>
+ <para>
+ Path to reach the current context from TopMemoryContext
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index 92ca5b2f72..81cb35dd47 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -20,6 +20,7 @@
#include "mb/pg_wchar.h"
#include "storage/proc.h"
#include "storage/procarray.h"
+#include "utils/array.h"
#include "utils/builtins.h"
/* ----------
@@ -28,6 +29,8 @@
*/
#define MEMORY_CONTEXT_IDENT_DISPLAY_SIZE 1024
+static Datum convert_path_to_datum(List *path);
+
/*
* PutMemoryContextsStatsTupleStore
* One recursion level for pg_get_backend_memory_contexts.
@@ -35,9 +38,10 @@
static void
PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
TupleDesc tupdesc, MemoryContext context,
- const char *parent, int level)
+ const char *parent, int level, int *context_id,
+ int parent_id, List *path)
{
-#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 9
+#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 12
Datum values[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
bool nulls[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
@@ -45,6 +49,7 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
MemoryContext child;
const char *name;
const char *ident;
+ int current_context_id = (*context_id)++;
Assert(MemoryContextIsValid(context));
@@ -103,13 +108,29 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
values[6] = Int64GetDatum(stat.freespace);
values[7] = Int64GetDatum(stat.freechunks);
values[8] = Int64GetDatum(stat.totalspace - stat.freespace);
+ values[9] = Int32GetDatum(current_context_id);
+
+ if(parent_id < 0)
+ /* TopMemoryContext has no parent context */
+ nulls[10] = true;
+ else
+ values[10] = Int32GetDatum(parent_id);
+
+ if (path == NIL)
+ nulls[11] = true;
+ else
+ values[11] = convert_path_to_datum(path);
+
tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+ path = lappend_int(path, current_context_id);
for (child = context->firstchild; child != NULL; child = child->nextchild)
{
- PutMemoryContextsStatsTupleStore(tupstore, tupdesc,
- child, name, level + 1);
+ PutMemoryContextsStatsTupleStore(tupstore, tupdesc, child, name,
+ level+1, context_id,
+ current_context_id, path);
}
+ path = list_delete_last(path);
}
/*
@@ -120,10 +141,15 @@ Datum
pg_get_backend_memory_contexts(PG_FUNCTION_ARGS)
{
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+ int context_id = 0;
+ List *path = NIL;
+
+ elog(LOG, "pg_get_backend_memory_contexts called");
InitMaterializedSRF(fcinfo, 0);
PutMemoryContextsStatsTupleStore(rsinfo->setResult, rsinfo->setDesc,
- TopMemoryContext, NULL, 0);
+ TopMemoryContext, NULL, 0, &context_id,
+ -1, path);
return (Datum) 0;
}
@@ -193,3 +219,26 @@ pg_log_backend_memory_contexts(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(true);
}
+
+/*
+ * Convert a list of context ids to a int[] Datum
+ */
+static Datum
+convert_path_to_datum(List *path)
+{
+ Datum *datum_array;
+ int length;
+ ArrayType *result_array;
+ ListCell *lc;
+
+ length = list_length(path);
+ datum_array = (Datum *) palloc(length * sizeof(Datum));
+ length = 0;
+ foreach(lc, path)
+ {
+ datum_array[length++] = Int32GetDatum((int) lfirst_int(lc));
+ }
+ result_array = construct_array_builtin(datum_array, length, INT4OID);
+
+ return PointerGetDatum(result_array);
+}
\ No newline at end of file
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 6996073989..49514ad792 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8212,9 +8212,9 @@
proname => 'pg_get_backend_memory_contexts', prorows => '100',
proretset => 't', provolatile => 'v', proparallel => 'r',
prorettype => 'record', proargtypes => '',
- proallargtypes => '{text,text,text,int4,int8,int8,int8,int8,int8}',
- proargmodes => '{o,o,o,o,o,o,o,o,o}',
- proargnames => '{name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes}',
+ proallargtypes => '{text,text,text,int4,int8,int8,int8,int8,int8,int4,int4,_int4}',
+ proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o}',
+ proargnames => '{name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes, id, parent_id, path}',
prosrc => 'pg_get_backend_memory_contexts' },
# logging memory contexts of the specified backend
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index e07afcd4aa..c7112be09e 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1311,8 +1311,11 @@ pg_backend_memory_contexts| SELECT name,
total_nblocks,
free_bytes,
free_chunks,
- used_bytes
- FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes);
+ used_bytes,
+ id,
+ parent_id,
+ path
+ FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes, id, parent_id, path);
pg_config| SELECT name,
setting
FROM pg_config() pg_config(name, setting);
--
2.25.1
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
@ 2023-10-12 16:23 ` Andres Freund <[email protected]>
2023-10-23 12:02 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
1 sibling, 1 reply; 19+ messages in thread
From: Andres Freund @ 2023-10-12 16:23 UTC (permalink / raw)
To: Melih Mutlu <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Hi,
On 2023-08-04 21:16:49 +0300, Melih Mutlu wrote:
> Melih Mutlu <[email protected]>, 16 Haz 2023 Cum, 17:03 tarihinde şunu
> yazdı:
>
> > With this change, here's a query to find how much space used by each
> > context including its children:
> >
> > > WITH RECURSIVE cte AS (
> > > SELECT id, total_bytes, id as root, name as root_name
> > > FROM memory_contexts
> > > UNION ALL
> > > SELECT r.id, r.total_bytes, cte.root, cte.root_name
> > > FROM memory_contexts r
> > > INNER JOIN cte ON r.parent_id = cte.id
> > > ),
> > > memory_contexts AS (
> > > SELECT * FROM pg_backend_memory_contexts
> > > )
> > > SELECT root as id, root_name as name, sum(total_bytes)
> > > FROM cte
> > > GROUP BY root, root_name
> > > ORDER BY sum DESC;
> >
>
> Given that the above query to get total bytes including all children is
> still a complex one, I decided to add an additional info in
> pg_backend_memory_contexts.
> The new "path" field displays an integer array that consists of ids of all
> parents for the current context. This way it's easier to tell whether a
> context is a child of another context, and we don't need to use recursive
> queries to get this info.
I think that does make it a good bit easier. Both to understand and to use.
> Here how pg_backend_memory_contexts would look like with this patch:
>
> postgres=# SELECT name, id, parent, parent_id, path
> FROM pg_backend_memory_contexts
> ORDER BY total_bytes DESC LIMIT 10;
> name | id | parent | parent_id | path
> -------------------------+-----+------------------+-----------+--------------
> CacheMemoryContext | 27 | TopMemoryContext | 0 | {0}
> Timezones | 124 | TopMemoryContext | 0 | {0}
> TopMemoryContext | 0 | | |
> MessageContext | 8 | TopMemoryContext | 0 | {0}
> WAL record construction | 118 | TopMemoryContext | 0 | {0}
> ExecutorState | 18 | PortalContext | 17 | {0,16,17}
> TupleSort main | 19 | ExecutorState | 18 | {0,16,17,18}
> TransactionAbortContext | 14 | TopMemoryContext | 0 | {0}
> smgr relation table | 10 | TopMemoryContext | 0 | {0}
> GUC hash table | 123 | GUCMemoryContext | 122 | {0,122}
> (10 rows)
Would we still need the parent_id column?
> +
> + <row>
> + <entry role="catalog_table_entry"><para role="column_definition">
> + <structfield>context_id</structfield> <type>int4</type>
> + </para>
> + <para>
> + Current context id
> + </para></entry>
> + </row>
I think the docs here need to warn that the id is ephemeral and will likely
differ in the next invocation.
> + <row>
> + <entry role="catalog_table_entry"><para role="column_definition">
> + <structfield>parent_id</structfield> <type>int4</type>
> + </para>
> + <para>
> + Parent context id
> + </para></entry>
> + </row>
> +
> + <row>
> + <entry role="catalog_table_entry"><para role="column_definition">
> + <structfield>path</structfield> <type>int4</type>
> + </para>
> + <para>
> + Path to reach the current context from TopMemoryContext
> + </para></entry>
> + </row>
Perhaps we should include some hint here how it could be used?
> </tbody>
> </tgroup>
> </table>
> diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
> index 92ca5b2f72..81cb35dd47 100644
> --- a/src/backend/utils/adt/mcxtfuncs.c
> +++ b/src/backend/utils/adt/mcxtfuncs.c
> @@ -20,6 +20,7 @@
> #include "mb/pg_wchar.h"
> #include "storage/proc.h"
> #include "storage/procarray.h"
> +#include "utils/array.h"
> #include "utils/builtins.h"
>
> /* ----------
> @@ -28,6 +29,8 @@
> */
> #define MEMORY_CONTEXT_IDENT_DISPLAY_SIZE 1024
>
> +static Datum convert_path_to_datum(List *path);
> +
> /*
> * PutMemoryContextsStatsTupleStore
> * One recursion level for pg_get_backend_memory_contexts.
> @@ -35,9 +38,10 @@
> static void
> PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
> TupleDesc tupdesc, MemoryContext context,
> - const char *parent, int level)
> + const char *parent, int level, int *context_id,
> + int parent_id, List *path)
> {
> -#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 9
> +#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 12
>
> Datum values[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
> bool nulls[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
> @@ -45,6 +49,7 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
> MemoryContext child;
> const char *name;
> const char *ident;
> + int current_context_id = (*context_id)++;
>
> Assert(MemoryContextIsValid(context));
>
> @@ -103,13 +108,29 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
> values[6] = Int64GetDatum(stat.freespace);
> values[7] = Int64GetDatum(stat.freechunks);
> values[8] = Int64GetDatum(stat.totalspace - stat.freespace);
> + values[9] = Int32GetDatum(current_context_id);
> +
> + if(parent_id < 0)
> + /* TopMemoryContext has no parent context */
> + nulls[10] = true;
> + else
> + values[10] = Int32GetDatum(parent_id);
> +
> + if (path == NIL)
> + nulls[11] = true;
> + else
> + values[11] = convert_path_to_datum(path);
> +
> tuplestore_putvalues(tupstore, tupdesc, values, nulls);
>
> + path = lappend_int(path, current_context_id);
> for (child = context->firstchild; child != NULL; child = child->nextchild)
> {
> - PutMemoryContextsStatsTupleStore(tupstore, tupdesc,
> - child, name, level + 1);
> + PutMemoryContextsStatsTupleStore(tupstore, tupdesc, child, name,
> + level+1, context_id,
> + current_context_id, path);
> }
> + path = list_delete_last(path);
> }
>
> /*
> @@ -120,10 +141,15 @@ Datum
> pg_get_backend_memory_contexts(PG_FUNCTION_ARGS)
> {
> ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
> + int context_id = 0;
> + List *path = NIL;
> +
> + elog(LOG, "pg_get_backend_memory_contexts called");
>
> InitMaterializedSRF(fcinfo, 0);
> PutMemoryContextsStatsTupleStore(rsinfo->setResult, rsinfo->setDesc,
> - TopMemoryContext, NULL, 0);
> + TopMemoryContext, NULL, 0, &context_id,
> + -1, path);
>
> return (Datum) 0;
> }
> @@ -193,3 +219,26 @@ pg_log_backend_memory_contexts(PG_FUNCTION_ARGS)
>
> PG_RETURN_BOOL(true);
> }
> +
> +/*
> + * Convert a list of context ids to a int[] Datum
> + */
> +static Datum
> +convert_path_to_datum(List *path)
> +{
> + Datum *datum_array;
> + int length;
> + ArrayType *result_array;
> + ListCell *lc;
> +
> + length = list_length(path);
> + datum_array = (Datum *) palloc(length * sizeof(Datum));
> + length = 0;
> + foreach(lc, path)
> + {
> + datum_array[length++] = Int32GetDatum((int) lfirst_int(lc));
The "(int)" in front of lfirst_int() seems redundant?
I think it'd be good to have some minimal test for this. E.g. checking that
there's multiple contexts below cache memory context or such.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
@ 2023-10-23 12:02 ` Melih Mutlu <[email protected]>
2023-12-04 04:43 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Melih Mutlu @ 2023-10-23 12:02 UTC (permalink / raw)
To: Andres Freund <[email protected]>; Stephen Frost <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Hi,
Thanks for reviewing.
Attached the updated patch v3.
Andres Freund <[email protected]>, 12 Eki 2023 Per, 19:23 tarihinde şunu
yazdı:
> > Here how pg_backend_memory_contexts would look like with this patch:
> >
> > postgres=# SELECT name, id, parent, parent_id, path
> > FROM pg_backend_memory_contexts
> > ORDER BY total_bytes DESC LIMIT 10;
> > name | id | parent | parent_id | path
> >
> -------------------------+-----+------------------+-----------+--------------
> > CacheMemoryContext | 27 | TopMemoryContext | 0 | {0}
> > Timezones | 124 | TopMemoryContext | 0 | {0}
> > TopMemoryContext | 0 | | |
> > MessageContext | 8 | TopMemoryContext | 0 | {0}
> > WAL record construction | 118 | TopMemoryContext | 0 | {0}
> > ExecutorState | 18 | PortalContext | 17 | {0,16,17}
> > TupleSort main | 19 | ExecutorState | 18 |
> {0,16,17,18}
> > TransactionAbortContext | 14 | TopMemoryContext | 0 | {0}
> > smgr relation table | 10 | TopMemoryContext | 0 | {0}
> > GUC hash table | 123 | GUCMemoryContext | 122 | {0,122}
> > (10 rows)
>
> Would we still need the parent_id column?
>
I guess not. Assuming the path column is sorted from TopMemoryContext to
the parent one level above, parent_id can be found using the path column if
needed.
Removed parent_id.
> > +
> > + <row>
> > + <entry role="catalog_table_entry"><para role="column_definition">
> > + <structfield>context_id</structfield> <type>int4</type>
> > + </para>
> > + <para>
> > + Current context id
> > + </para></entry>
> > + </row>
>
> I think the docs here need to warn that the id is ephemeral and will likely
> differ in the next invocation.
>
Done.
> + <row>
> > + <entry role="catalog_table_entry"><para role="column_definition">
> > + <structfield>parent_id</structfield> <type>int4</type>
> > + </para>
> > + <para>
> > + Parent context id
> > + </para></entry>
> > + </row>
> > +
> > + <row>
> > + <entry role="catalog_table_entry"><para role="column_definition">
> > + <structfield>path</structfield> <type>int4</type>
> > + </para>
> > + <para>
> > + Path to reach the current context from TopMemoryContext
> > + </para></entry>
> > + </row>
>
> Perhaps we should include some hint here how it could be used?
>
I added more explanation but not sure if that is what you asked for. Do you
want a hint that is related to a more specific use case?
> + length = list_length(path);
> > + datum_array = (Datum *) palloc(length * sizeof(Datum));
> > + length = 0;
> > + foreach(lc, path)
> > + {
> > + datum_array[length++] = Int32GetDatum((int)
> lfirst_int(lc));
>
> The "(int)" in front of lfirst_int() seems redundant?
>
Removed.
I think it'd be good to have some minimal test for this. E.g. checking that
> there's multiple contexts below cache memory context or such.
>
Added new tests in sysview.sql.
Stephen Frost <[email protected]>, 18 Eki 2023 Çar, 22:53 tarihinde şunu
yazdı:
> I wonder if we should perhaps just include
> "total_bytes_including_children" as another column? Certainly seems
> like a very useful thing that folks would like to see. We could do that
> either with C, or even something as simple as changing the view to do
> something like:
>
> WITH contexts AS MATERIALIZED (
> SELECT * FROM pg_get_backend_memory_contexts()
> )
> SELECT
> *,
> coalesce
> (
> (
> (SELECT sum(total_bytes) FROM contexts WHERE ARRAY[a.id] <@ path)
> + total_bytes
> ),
> total_bytes
> ) AS total_bytes_including_children
> FROM contexts a;
>
I added a "total_bytes_including_children" column as you suggested. Did
that with C since it seemed faster than doing it by changing the view.
-- Calculating total_bytes_including_children by modifying the view
postgres=# select * from pg_backend_memory_contexts ;
Time: 30.462 ms
-- Calculating total_bytes_including_children with C
postgres=# select * from pg_backend_memory_contexts ;
Time: 1.511 ms
Thanks,
--
Melih Mutlu
Microsoft
Attachments:
[application/octet-stream] v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch (10.7K, ../../CAGPVpCQjpcU=T9wtzWX+dU5LkXnPc2OBqNHoZmKHWFFH8YL7ZQ@mail.gmail.com/3-v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch)
download | inline diff:
From 4595e8e31f7c234426e7f104890cd3f0f0e4ca10 Mon Sep 17 00:00:00 2001
From: Melih Mutlu <[email protected]>
Date: Tue, 13 Jun 2023 15:00:05 +0300
Subject: [PATCH v3] Adding id/parent_id into pg_backend_memory_contexts
Added three new fields into the tuples returned by
pg_get_backend_memory_contexts() to specify the parent/child relation
between memory contexts and the path from root to current context.
Context names cannot be relied on since they're not unique. Therefore,
unique id and parent_id are needed for each context. Those new id's are
assigned during pg_get_backend_memory_contexts() call and not stored
anywhere. So they may change in each pg_get_backend_memory_contexts()
call and shouldn't be used across differenct
pg_get_backend_memory_contexts() calls.
Context id's are start from 0 which is assigned to TopMemoryContext.
---
doc/src/sgml/system-views.sgml | 30 +++++++++++++
src/backend/utils/adt/mcxtfuncs.c | 62 +++++++++++++++++++++++---
src/include/catalog/pg_proc.dat | 6 +--
src/test/regress/expected/rules.out | 7 ++-
src/test/regress/expected/sysviews.out | 23 +++++++++-
src/test/regress/sql/sysviews.sql | 15 ++++++-
6 files changed, 130 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 2b35c2f91b..29db104c81 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -543,6 +543,36 @@
Used space in bytes
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>context_id</structfield> <type>int4</type>
+ </para>
+ <para>
+ Current context id. Note that the context id is a temporary id and may
+ change in each invocation
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>path</structfield> <type>int4</type>
+ </para>
+ <para>
+ Path to reach the current context from TopMemoryContext. Context ids in
+ this list represents all parents of the current context. This can be
+ used to build the parent and child relation.
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>total_bytes_including_children</structfield> <type>int8</type>
+ </para>
+ <para>
+ Total bytes allocated for this memory context including its children
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index 92ca5b2f72..9e7aa3535b 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -20,6 +20,7 @@
#include "mb/pg_wchar.h"
#include "storage/proc.h"
#include "storage/procarray.h"
+#include "utils/array.h"
#include "utils/builtins.h"
/* ----------
@@ -28,6 +29,8 @@
*/
#define MEMORY_CONTEXT_IDENT_DISPLAY_SIZE 1024
+static Datum convert_path_to_datum(List *path);
+
/*
* PutMemoryContextsStatsTupleStore
* One recursion level for pg_get_backend_memory_contexts.
@@ -35,9 +38,10 @@
static void
PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
TupleDesc tupdesc, MemoryContext context,
- const char *parent, int level)
+ const char *parent, int level, int *context_id,
+ List *path, Size *total_bytes_inc_chidlren)
{
-#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 9
+#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 12
Datum values[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
bool nulls[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
@@ -45,6 +49,8 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
MemoryContext child;
const char *name;
const char *ident;
+ int current_context_id = (*context_id)++;
+ Size total_bytes_children = 0;
Assert(MemoryContextIsValid(context));
@@ -103,13 +109,28 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
values[6] = Int64GetDatum(stat.freespace);
values[7] = Int64GetDatum(stat.freechunks);
values[8] = Int64GetDatum(stat.totalspace - stat.freespace);
- tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+ values[9] = Int32GetDatum(current_context_id);
+
+ if (path == NIL)
+ nulls[10] = true;
+ else
+ values[10] = convert_path_to_datum(path);
+ path = lappend_int(path, current_context_id);
for (child = context->firstchild; child != NULL; child = child->nextchild)
{
- PutMemoryContextsStatsTupleStore(tupstore, tupdesc,
- child, name, level + 1);
+ PutMemoryContextsStatsTupleStore(tupstore, tupdesc, child, name,
+ level+1, context_id, path,
+ &total_bytes_children);
}
+ path = list_delete_last(path);
+
+ total_bytes_children += stat.totalspace;
+ values[11] = Int64GetDatum(total_bytes_children);
+
+ tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+
+ *total_bytes_inc_chidlren += total_bytes_children;
}
/*
@@ -120,10 +141,16 @@ Datum
pg_get_backend_memory_contexts(PG_FUNCTION_ARGS)
{
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+ int context_id = 0;
+ List *path = NIL;
+ Size total_bytes_inc_chidlren = 0;
+
+ elog(LOG, "pg_get_backend_memory_contexts called");
InitMaterializedSRF(fcinfo, 0);
PutMemoryContextsStatsTupleStore(rsinfo->setResult, rsinfo->setDesc,
- TopMemoryContext, NULL, 0);
+ TopMemoryContext, NULL, 0, &context_id,
+ path, &total_bytes_inc_chidlren);
return (Datum) 0;
}
@@ -193,3 +220,26 @@ pg_log_backend_memory_contexts(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(true);
}
+
+/*
+ * Convert a list of context ids to a int[] Datum
+ */
+static Datum
+convert_path_to_datum(List *path)
+{
+ Datum *datum_array;
+ int length;
+ ArrayType *result_array;
+ ListCell *lc;
+
+ length = list_length(path);
+ datum_array = (Datum *) palloc(length * sizeof(Datum));
+ length = 0;
+ foreach(lc, path)
+ {
+ datum_array[length++] = Int32GetDatum(lfirst_int(lc));
+ }
+ result_array = construct_array_builtin(datum_array, length, INT4OID);
+
+ return PointerGetDatum(result_array);
+}
\ No newline at end of file
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index c92d0631a0..789ba25fa3 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8239,9 +8239,9 @@
proname => 'pg_get_backend_memory_contexts', prorows => '100',
proretset => 't', provolatile => 'v', proparallel => 'r',
prorettype => 'record', proargtypes => '',
- proallargtypes => '{text,text,text,int4,int8,int8,int8,int8,int8}',
- proargmodes => '{o,o,o,o,o,o,o,o,o}',
- proargnames => '{name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes}',
+ proallargtypes => '{text,text,text,int4,int8,int8,int8,int8,int8,int4,_int4,int8}',
+ proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o}',
+ proargnames => '{name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes, id, path, total_bytes_including_children}',
prosrc => 'pg_get_backend_memory_contexts' },
# logging memory contexts of the specified backend
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 2c60400ade..8fd3e9f6fd 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1311,8 +1311,11 @@ pg_backend_memory_contexts| SELECT name,
total_nblocks,
free_bytes,
free_chunks,
- used_bytes
- FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes);
+ used_bytes,
+ id,
+ path,
+ total_bytes_including_children
+ FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes, id, path, total_bytes_including_children);
pg_config| SELECT name,
setting
FROM pg_config() pg_config(name, setting);
diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out
index aae5d51e1c..45d8064c35 100644
--- a/src/test/regress/expected/sysviews.out
+++ b/src/test/regress/expected/sysviews.out
@@ -20,7 +20,7 @@ select count(*) >= 0 as ok from pg_available_extensions;
(1 row)
-- The entire output of pg_backend_memory_contexts is not stable,
--- we test only the existence and basic condition of TopMemoryContext.
+-- we test the existence and basic condition of TopMemoryContext.
select name, ident, parent, level, total_bytes >= free_bytes
from pg_backend_memory_contexts where level = 0;
name | ident | parent | level | ?column?
@@ -28,6 +28,27 @@ select name, ident, parent, level, total_bytes >= free_bytes
TopMemoryContext | | | 0 | t
(1 row)
+-- Test whether there are contexts with CacheMemoryContext in their path.
+-- There should be multiple children of CacheMemoryContext.
+with contexts as (
+ select * from pg_backend_memory_contexts
+)
+select count(*) > 0
+from contexts
+where array[(select id from contexts where name = 'CacheMemoryContext')] <@ path;
+ ?column?
+----------
+ t
+(1 row)
+
+-- TopMemoryContext should have the largest total_bytes_including_children.
+select name from pg_backend_memory_contexts
+ order by total_bytes_including_children desc limit 1;
+ name
+------------------
+ TopMemoryContext
+(1 row)
+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
ok
diff --git a/src/test/regress/sql/sysviews.sql b/src/test/regress/sql/sysviews.sql
index 6b4e24601d..56cfcb77f4 100644
--- a/src/test/regress/sql/sysviews.sql
+++ b/src/test/regress/sql/sysviews.sql
@@ -13,10 +13,23 @@ select count(*) >= 0 as ok from pg_available_extension_versions;
select count(*) >= 0 as ok from pg_available_extensions;
-- The entire output of pg_backend_memory_contexts is not stable,
--- we test only the existence and basic condition of TopMemoryContext.
+-- we test the existence and basic condition of TopMemoryContext.
select name, ident, parent, level, total_bytes >= free_bytes
from pg_backend_memory_contexts where level = 0;
+-- Test whether there are contexts with CacheMemoryContext in their path.
+-- There should be multiple children of CacheMemoryContext.
+with contexts as (
+ select * from pg_backend_memory_contexts
+)
+select count(*) > 0
+from contexts
+where array[(select id from contexts where name = 'CacheMemoryContext')] <@ path;
+
+-- TopMemoryContext should have the largest total_bytes_including_children.
+select name from pg_backend_memory_contexts
+ order by total_bytes_including_children desc limit 1;
+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
--
2.34.1
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
2023-10-23 12:02 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
@ 2023-12-04 04:43 ` torikoshia <[email protected]>
2024-01-03 11:40 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: torikoshia @ 2023-12-04 04:43 UTC (permalink / raw)
To: Melih Mutlu <[email protected]>; +Cc: Andres Freund <[email protected]>; Stephen Frost <[email protected]>; PostgreSQL Hackers <[email protected]>
Thanks for working on this improvement!
On 2023-10-23 21:02, Melih Mutlu wrote:
> Hi,
>
> Thanks for reviewing.
> Attached the updated patch v3.
I reviewed v3 patch and here are some minor comments:
> + <row>
> + <entry role="catalog_table_entry"><para
> role="column_definition">
> + <structfield>path</structfield> <type>int4</type>
Should 'int4' be 'int4[]'?
Other system catalog columns such as pg_groups.grolist distinguish
whther the type is a array or not.
> + Path to reach the current context from TopMemoryContext.
> Context ids in
> + this list represents all parents of the current context. This
> can be
> + used to build the parent and child relation.
It seems last "." is not necessary considering other explanations for
each field end without it.
+ const char *parent, int level, int
*context_id,
+ List *path, Size
*total_bytes_inc_chidlren)
'chidlren' -> 'children'
+ elog(LOG, "pg_get_backend_memory_contexts called");
Is this message necessary?
There was warning when applying the patch:
% git apply
../patch/pg_backend_memory_context_refine/v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch
../patch/pg_backend_memory_context_refine/v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch:282:
trailing whitespace.
select count(*) > 0
../patch/pg_backend_memory_context_refine/v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch:283:
trailing whitespace.
from contexts
warning: 2 lines add whitespace errors.
--
Regards,
--
Atsushi Torikoshi
NTT DATA Group Corporation
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
2023-10-23 12:02 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-12-04 04:43 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
@ 2024-01-03 11:40 ` Melih Mutlu <[email protected]>
2024-01-10 06:37 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Melih Mutlu @ 2024-01-03 11:40 UTC (permalink / raw)
To: torikoshia <[email protected]>; +Cc: Andres Freund <[email protected]>; Stephen Frost <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
Thanks for reviewing. Please find the updated patch attached.
torikoshia <[email protected]>, 4 Ara 2023 Pzt, 07:43 tarihinde
şunu yazdı:
> I reviewed v3 patch and here are some minor comments:
>
> > + <row>
> > + <entry role="catalog_table_entry"><para
> > role="column_definition">
> > + <structfield>path</structfield> <type>int4</type>
>
> Should 'int4' be 'int4[]'?
> Other system catalog columns such as pg_groups.grolist distinguish
> whther the type is a array or not.
>
Right! Done.
>
> > + Path to reach the current context from TopMemoryContext.
> > Context ids in
> > + this list represents all parents of the current context. This
> > can be
> > + used to build the parent and child relation.
>
> It seems last "." is not necessary considering other explanations for
> each field end without it.
>
Done.
> + const char *parent, int level, int
> *context_id,
> + List *path, Size
> *total_bytes_inc_chidlren)
>
> 'chidlren' -> 'children'
>
Done.
> + elog(LOG, "pg_get_backend_memory_contexts called");
>
> Is this message necessary?
>
I guess I added this line for debugging and then forgot to remove. Now
removed.
There was warning when applying the patch:
>
> % git apply
>
> ../patch/pg_backend_memory_context_refine/v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch
>
> ../patch/pg_backend_memory_context_refine/v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch:282:
>
> trailing whitespace.
> select count(*) > 0
>
> ../patch/pg_backend_memory_context_refine/v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch:283:
>
> trailing whitespace.
> from contexts
> warning: 2 lines add whitespace errors.
>
Fixed.
Thanks,
--
Melih Mutlu
Microsoft
Attachments:
[application/octet-stream] v4-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch (10.6K, ../../CAGPVpCT0xuoBvx0=myzKYmv02bQ64BC96Z_-dVKckH3++Jg+DA@mail.gmail.com/3-v4-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch)
download | inline diff:
From 8971207cc727870f4fbaad76579f901247fb0c1d Mon Sep 17 00:00:00 2001
From: Melih Mutlu <[email protected]>
Date: Tue, 13 Jun 2023 15:00:05 +0300
Subject: [PATCH v4] Adding id/parent_id into pg_backend_memory_contexts
Added three new fields into the tuples returned by
pg_get_backend_memory_contexts() to specify the parent/child relation
between memory contexts and the path from root to current context.
Context names cannot be relied on since they're not unique. Therefore,
unique id and parent_id are needed for each context. Those new id's are
assigned during pg_get_backend_memory_contexts() call and not stored
anywhere. So they may change in each pg_get_backend_memory_contexts()
call and shouldn't be used across differenct
pg_get_backend_memory_contexts() calls.
Context id's are start from 0 which is assigned to TopMemoryContext.
---
doc/src/sgml/system-views.sgml | 30 +++++++++++++
src/backend/utils/adt/mcxtfuncs.c | 60 +++++++++++++++++++++++---
src/include/catalog/pg_proc.dat | 6 +--
src/test/regress/expected/rules.out | 7 ++-
src/test/regress/expected/sysviews.out | 23 +++++++++-
src/test/regress/sql/sysviews.sql | 15 ++++++-
6 files changed, 128 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 0ef1745631..488e9df4f6 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -543,6 +543,36 @@
Used space in bytes
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>context_id</structfield> <type>int4</type>
+ </para>
+ <para>
+ Current context id. Note that the context id is a temporary id and may
+ change in each invocation
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>path</structfield> <type>int4[]</type>
+ </para>
+ <para>
+ Path to reach the current context from TopMemoryContext. Context ids in
+ this list represents all parents of the current context. This can be
+ used to build the parent and child relation
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>total_bytes_including_children</structfield> <type>int8</type>
+ </para>
+ <para>
+ Total bytes allocated for this memory context including its children
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index 92ca5b2f72..bc357a083b 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -20,6 +20,7 @@
#include "mb/pg_wchar.h"
#include "storage/proc.h"
#include "storage/procarray.h"
+#include "utils/array.h"
#include "utils/builtins.h"
/* ----------
@@ -28,6 +29,8 @@
*/
#define MEMORY_CONTEXT_IDENT_DISPLAY_SIZE 1024
+static Datum convert_path_to_datum(List *path);
+
/*
* PutMemoryContextsStatsTupleStore
* One recursion level for pg_get_backend_memory_contexts.
@@ -35,9 +38,10 @@
static void
PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
TupleDesc tupdesc, MemoryContext context,
- const char *parent, int level)
+ const char *parent, int level, int *context_id,
+ List *path, Size *total_bytes_inc_children)
{
-#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 9
+#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 12
Datum values[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
bool nulls[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
@@ -45,6 +49,8 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
MemoryContext child;
const char *name;
const char *ident;
+ int current_context_id = (*context_id)++;
+ Size total_bytes_children = 0;
Assert(MemoryContextIsValid(context));
@@ -103,13 +109,28 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
values[6] = Int64GetDatum(stat.freespace);
values[7] = Int64GetDatum(stat.freechunks);
values[8] = Int64GetDatum(stat.totalspace - stat.freespace);
- tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+ values[9] = Int32GetDatum(current_context_id);
+
+ if (path == NIL)
+ nulls[10] = true;
+ else
+ values[10] = convert_path_to_datum(path);
+ path = lappend_int(path, current_context_id);
for (child = context->firstchild; child != NULL; child = child->nextchild)
{
- PutMemoryContextsStatsTupleStore(tupstore, tupdesc,
- child, name, level + 1);
+ PutMemoryContextsStatsTupleStore(tupstore, tupdesc, child, name,
+ level+1, context_id, path,
+ &total_bytes_children);
}
+ path = list_delete_last(path);
+
+ total_bytes_children += stat.totalspace;
+ values[11] = Int64GetDatum(total_bytes_children);
+
+ tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+
+ *total_bytes_inc_children += total_bytes_children;
}
/*
@@ -120,10 +141,14 @@ Datum
pg_get_backend_memory_contexts(PG_FUNCTION_ARGS)
{
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+ int context_id = 0;
+ List *path = NIL;
+ Size total_bytes_inc_children = 0;
InitMaterializedSRF(fcinfo, 0);
PutMemoryContextsStatsTupleStore(rsinfo->setResult, rsinfo->setDesc,
- TopMemoryContext, NULL, 0);
+ TopMemoryContext, NULL, 0, &context_id,
+ path, &total_bytes_inc_children);
return (Datum) 0;
}
@@ -193,3 +218,26 @@ pg_log_backend_memory_contexts(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(true);
}
+
+/*
+ * Convert a list of context ids to a int[] Datum
+ */
+static Datum
+convert_path_to_datum(List *path)
+{
+ Datum *datum_array;
+ int length;
+ ArrayType *result_array;
+ ListCell *lc;
+
+ length = list_length(path);
+ datum_array = (Datum *) palloc(length * sizeof(Datum));
+ length = 0;
+ foreach(lc, path)
+ {
+ datum_array[length++] = Int32GetDatum(lfirst_int(lc));
+ }
+ result_array = construct_array_builtin(datum_array, length, INT4OID);
+
+ return PointerGetDatum(result_array);
+}
\ No newline at end of file
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 5b67784731..51280c563d 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8263,9 +8263,9 @@
proname => 'pg_get_backend_memory_contexts', prorows => '100',
proretset => 't', provolatile => 'v', proparallel => 'r',
prorettype => 'record', proargtypes => '',
- proallargtypes => '{text,text,text,int4,int8,int8,int8,int8,int8}',
- proargmodes => '{o,o,o,o,o,o,o,o,o}',
- proargnames => '{name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes}',
+ proallargtypes => '{text,text,text,int4,int8,int8,int8,int8,int8,int4,_int4,int8}',
+ proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o}',
+ proargnames => '{name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes, id, path, total_bytes_including_children}',
prosrc => 'pg_get_backend_memory_contexts' },
# logging memory contexts of the specified backend
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index f645e8486b..cf28820b26 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1311,8 +1311,11 @@ pg_backend_memory_contexts| SELECT name,
total_nblocks,
free_bytes,
free_chunks,
- used_bytes
- FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes);
+ used_bytes,
+ id,
+ path,
+ total_bytes_including_children
+ FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes, id, path, total_bytes_including_children);
pg_config| SELECT name,
setting
FROM pg_config() pg_config(name, setting);
diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out
index 271313ebf8..de9f1ce62c 100644
--- a/src/test/regress/expected/sysviews.out
+++ b/src/test/regress/expected/sysviews.out
@@ -20,7 +20,7 @@ select count(*) >= 0 as ok from pg_available_extensions;
(1 row)
-- The entire output of pg_backend_memory_contexts is not stable,
--- we test only the existence and basic condition of TopMemoryContext.
+-- we test the existence and basic condition of TopMemoryContext.
select name, ident, parent, level, total_bytes >= free_bytes
from pg_backend_memory_contexts where level = 0;
name | ident | parent | level | ?column?
@@ -28,6 +28,27 @@ select name, ident, parent, level, total_bytes >= free_bytes
TopMemoryContext | | | 0 | t
(1 row)
+-- Test whether there are contexts with CacheMemoryContext in their path.
+-- There should be multiple children of CacheMemoryContext.
+with contexts as (
+ select * from pg_backend_memory_contexts
+)
+select count(*) > 0
+from contexts
+where array[(select id from contexts where name = 'CacheMemoryContext')] <@ path;
+ ?column?
+----------
+ t
+(1 row)
+
+-- TopMemoryContext should have the largest total_bytes_including_children.
+select name from pg_backend_memory_contexts
+ order by total_bytes_including_children desc limit 1;
+ name
+------------------
+ TopMemoryContext
+(1 row)
+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
ok
diff --git a/src/test/regress/sql/sysviews.sql b/src/test/regress/sql/sysviews.sql
index 6b4e24601d..1573e5012d 100644
--- a/src/test/regress/sql/sysviews.sql
+++ b/src/test/regress/sql/sysviews.sql
@@ -13,10 +13,23 @@ select count(*) >= 0 as ok from pg_available_extension_versions;
select count(*) >= 0 as ok from pg_available_extensions;
-- The entire output of pg_backend_memory_contexts is not stable,
--- we test only the existence and basic condition of TopMemoryContext.
+-- we test the existence and basic condition of TopMemoryContext.
select name, ident, parent, level, total_bytes >= free_bytes
from pg_backend_memory_contexts where level = 0;
+-- Test whether there are contexts with CacheMemoryContext in their path.
+-- There should be multiple children of CacheMemoryContext.
+with contexts as (
+ select * from pg_backend_memory_contexts
+)
+select count(*) > 0
+from contexts
+where array[(select id from contexts where name = 'CacheMemoryContext')] <@ path;
+
+-- TopMemoryContext should have the largest total_bytes_including_children.
+select name from pg_backend_memory_contexts
+ order by total_bytes_including_children desc limit 1;
+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
--
2.34.1
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
2023-10-23 12:02 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-12-04 04:43 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-03 11:40 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
@ 2024-01-10 06:37 ` torikoshia <[email protected]>
2024-01-16 09:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: torikoshia @ 2024-01-10 06:37 UTC (permalink / raw)
To: Melih Mutlu <[email protected]>; +Cc: Andres Freund <[email protected]>; Stephen Frost <[email protected]>; PostgreSQL Hackers <[email protected]>
On 2024-01-03 20:40, Melih Mutlu wrote:
> Hi,
>
> Thanks for reviewing. Please find the updated patch attached.
>
> torikoshia <[email protected]>, 4 Ara 2023 Pzt, 07:43
> tarihinde şunu yazdı:
>
>> I reviewed v3 patch and here are some minor comments:
>>
>>> + <row>
>>> + <entry role="catalog_table_entry"><para
>>> role="column_definition">
>>> + <structfield>path</structfield> <type>int4</type>
>>
>> Should 'int4' be 'int4[]'?
>> Other system catalog columns such as pg_groups.grolist distinguish
>> whther the type is a array or not.
>
> Right! Done.
>
>>> + Path to reach the current context from TopMemoryContext.
>>> Context ids in
>>> + this list represents all parents of the current context.
>> This
>>> can be
>>> + used to build the parent and child relation.
>>
>> It seems last "." is not necessary considering other explanations
>> for
>> each field end without it.
>
> Done.
>
>> + const char *parent, int level, int
>> *context_id,
>> + List *path, Size
>> *total_bytes_inc_chidlren)
>>
>> 'chidlren' -> 'children'
>
> Done.
>
>> + elog(LOG, "pg_get_backend_memory_contexts called");
>>
>> Is this message necessary?
>
> I guess I added this line for debugging and then forgot to remove. Now
> removed.
>
>> There was warning when applying the patch:
>>
>> % git apply
>>
> ../patch/pg_backend_memory_context_refine/v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch
>>
>>
> ../patch/pg_backend_memory_context_refine/v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch:282:
>>
>> trailing whitespace.
>> select count(*) > 0
>>
>>
> ../patch/pg_backend_memory_context_refine/v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch:283:
>>
>> trailing whitespace.
>> from contexts
>> warning: 2 lines add whitespace errors.
>
> Fixed.
>
> Thanks,--
>
> Melih Mutlu
> Microsoft
Thanks for updating the patch.
> + <row>
> + <entry role="catalog_table_entry"><para
> role="column_definition">
> + <structfield>context_id</structfield> <type>int4</type>
> + </para>
> + <para>
> + Current context id. Note that the context id is a temporary id
> and may
> + change in each invocation
> + </para></entry>
> + </row>
> +
> + <row>
> + <entry role="catalog_table_entry"><para
> role="column_definition">
> + <structfield>path</structfield> <type>int4[]</type>
> + </para>
> + <para>
> + Path to reach the current context from TopMemoryContext.
> Context ids in
> + this list represents all parents of the current context. This
> can be
> + used to build the parent and child relation
> + </para></entry>
> + </row>
> +
> + <row>
> + <entry role="catalog_table_entry"><para
> role="column_definition">
> + <structfield>total_bytes_including_children</structfield>
> <type>int8</type>
> + </para>
> + <para>
> + Total bytes allocated for this memory context including its
> children
> + </para></entry>
> + </row>
These columns are currently added to the bottom of the table, but it may
be better to put semantically similar items close together and change
the insertion position with reference to other system views. For
example,
- In pg_group and pg_user, 'id' is placed on the line following 'name',
so 'context_id' be placed on the line following 'name'
- 'path' is similar with 'parent' and 'level' in that these are
information about the location of the context, 'path' be placed to next
to them.
If we do this, orders of columns in the system view should be the same,
I think.
> + ListCell *lc;
> +
> + length = list_length(path);
> + datum_array = (Datum *) palloc(length * sizeof(Datum));
> + length = 0;
> + foreach(lc, path)
> + {
> + datum_array[length++] = Int32GetDatum(lfirst_int(lc));
> + }
14dd0f27d have introduced new macro foreach_int.
It seems to be able to make the code a bit simpler and the commit log
says this macro is primarily intended for use in new code. For example:
| int id;
|
| length = list_length(path);
| datum_array = (Datum *) palloc(length * sizeof(Datum));
| length = 0;
| foreach_int(id, path)
| {
| datum_array[length++] = Int32GetDatum(id);
| }
--
Regards,
--
Atsushi Torikoshi
NTT DATA Group Corporation
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
2023-10-23 12:02 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-12-04 04:43 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-03 11:40 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-10 06:37 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
@ 2024-01-16 09:41 ` Melih Mutlu <[email protected]>
2024-01-19 08:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Melih Mutlu @ 2024-01-16 09:41 UTC (permalink / raw)
To: torikoshia <[email protected]>; +Cc: Andres Freund <[email protected]>; Stephen Frost <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
Thanks for reviewing.
torikoshia <[email protected]>, 10 Oca 2024 Çar, 09:37 tarihinde
şunu yazdı:
> > + <row>
> > + <entry role="catalog_table_entry"><para
> > role="column_definition">
> > + <structfield>context_id</structfield> <type>int4</type>
> > + </para>
> > + <para>
> > + Current context id. Note that the context id is a temporary id
> > and may
> > + change in each invocation
> > + </para></entry>
> > + </row>
> > +
> > + <row>
> > + <entry role="catalog_table_entry"><para
> > role="column_definition">
> > + <structfield>path</structfield> <type>int4[]</type>
> > + </para>
> > + <para>
> > + Path to reach the current context from TopMemoryContext.
> > Context ids in
> > + this list represents all parents of the current context. This
> > can be
> > + used to build the parent and child relation
> > + </para></entry>
> > + </row>
> > +
> > + <row>
> > + <entry role="catalog_table_entry"><para
> > role="column_definition">
> > + <structfield>total_bytes_including_children</structfield>
> > <type>int8</type>
> > + </para>
> > + <para>
> > + Total bytes allocated for this memory context including its
> > children
> > + </para></entry>
> > + </row>
>
> These columns are currently added to the bottom of the table, but it may
> be better to put semantically similar items close together and change
> the insertion position with reference to other system views. For
> example,
>
> - In pg_group and pg_user, 'id' is placed on the line following 'name',
> so 'context_id' be placed on the line following 'name'
> - 'path' is similar with 'parent' and 'level' in that these are
> information about the location of the context, 'path' be placed to next
> to them.
>
> If we do this, orders of columns in the system view should be the same,
> I think.
>
I've done what you suggested. Also moved "total_bytes_including_children"
right after "total_bytes".
14dd0f27d have introduced new macro foreach_int.
> It seems to be able to make the code a bit simpler and the commit log
> says this macro is primarily intended for use in new code. For example:
>
Makes sense. Done.
Thanks,
--
Melih Mutlu
Microsoft
Attachments:
[application/octet-stream] v5-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch (12.2K, ../../CAGPVpCSvNDXy1vDTjgGVCOCAndQ6iB_LVyjb1GNATEqdx8zLHw@mail.gmail.com/3-v5-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch)
download | inline diff:
From 5a493bf26a27ed6ec9a465adedee7a0678f0138f Mon Sep 17 00:00:00 2001
From: Melih Mutlu <[email protected]>
Date: Tue, 13 Jun 2023 15:00:05 +0300
Subject: [PATCH v5] Adding id/parent_id into pg_backend_memory_contexts
Added three new fields into the tuples returned by
pg_get_backend_memory_contexts() to specify the parent/child relation
between memory contexts and the path from root to current context.
Context names cannot be relied on since they're not unique. Therefore,
unique id and parent_id are needed for each context. Those new id's are
assigned during pg_get_backend_memory_contexts() call and not stored
anywhere. So they may change in each pg_get_backend_memory_contexts()
call and shouldn't be used across differenct
pg_get_backend_memory_contexts() calls.
Context id's are start from 0 which is assigned to TopMemoryContext.
---
doc/src/sgml/system-views.sgml | 30 ++++++++++
src/backend/utils/adt/mcxtfuncs.c | 81 +++++++++++++++++++++-----
src/include/catalog/pg_proc.dat | 6 +-
src/test/regress/expected/rules.out | 5 +-
src/test/regress/expected/sysviews.out | 23 +++++++-
src/test/regress/sql/sysviews.sql | 15 ++++-
6 files changed, 138 insertions(+), 22 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 72d01fc624..1d7d94b61d 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -472,6 +472,16 @@
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>context_id</structfield> <type>int4</type>
+ </para>
+ <para>
+ Current context id. Note that the context id is a temporary id and may
+ change in each invocation
+ </para></entry>
+ </row>
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>ident</structfield> <type>text</type>
@@ -499,6 +509,17 @@
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>path</structfield> <type>int4[]</type>
+ </para>
+ <para>
+ Path to reach the current context from TopMemoryContext. Context ids in
+ this list represents all parents of the current context. This can be
+ used to build the parent and child relation
+ </para></entry>
+ </row>
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>total_bytes</structfield> <type>int8</type>
@@ -508,6 +529,15 @@
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>total_bytes_including_children</structfield> <type>int8</type>
+ </para>
+ <para>
+ Total bytes allocated for this memory context including its children
+ </para></entry>
+ </row>
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>total_nblocks</structfield> <type>int8</type>
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index 4708d73f5f..44e6b87fe0 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -20,6 +20,7 @@
#include "mb/pg_wchar.h"
#include "storage/proc.h"
#include "storage/procarray.h"
+#include "utils/array.h"
#include "utils/builtins.h"
/* ----------
@@ -28,6 +29,8 @@
*/
#define MEMORY_CONTEXT_IDENT_DISPLAY_SIZE 1024
+static Datum convert_path_to_datum(List *path);
+
/*
* PutMemoryContextsStatsTupleStore
* One recursion level for pg_get_backend_memory_contexts.
@@ -35,9 +38,10 @@
static void
PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
TupleDesc tupdesc, MemoryContext context,
- const char *parent, int level)
+ const char *parent, int level, int *context_id,
+ List *path, Size *total_bytes_inc_children)
{
-#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 9
+#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 12
Datum values[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
bool nulls[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
@@ -45,6 +49,8 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
MemoryContext child;
const char *name;
const char *ident;
+ int current_context_id = (*context_id)++;
+ Size total_bytes_children = 0;
Assert(MemoryContextIsValid(context));
@@ -73,6 +79,8 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
else
nulls[0] = true;
+ values[1] = Int32GetDatum(current_context_id);
+
if (ident)
{
int idlen = strlen(ident);
@@ -87,29 +95,44 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
memcpy(clipped_ident, ident, idlen);
clipped_ident[idlen] = '\0';
- values[1] = CStringGetTextDatum(clipped_ident);
+ values[2] = CStringGetTextDatum(clipped_ident);
}
else
- nulls[1] = true;
+ nulls[2] = true;
if (parent)
- values[2] = CStringGetTextDatum(parent);
+ values[3] = CStringGetTextDatum(parent);
else
- nulls[2] = true;
+ nulls[3] = true;
- values[3] = Int32GetDatum(level);
- values[4] = Int64GetDatum(stat.totalspace);
- values[5] = Int64GetDatum(stat.nblocks);
- values[6] = Int64GetDatum(stat.freespace);
- values[7] = Int64GetDatum(stat.freechunks);
- values[8] = Int64GetDatum(stat.totalspace - stat.freespace);
- tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+ values[4] = Int32GetDatum(level);
+ if (path == NIL)
+ nulls[5] = true;
+ else
+ values[5] = convert_path_to_datum(path);
+
+ values[6] = Int64GetDatum(stat.totalspace);
+
+ path = lappend_int(path, current_context_id);
for (child = context->firstchild; child != NULL; child = child->nextchild)
{
- PutMemoryContextsStatsTupleStore(tupstore, tupdesc,
- child, name, level + 1);
+ PutMemoryContextsStatsTupleStore(tupstore, tupdesc, child, name,
+ level+1, context_id, path,
+ &total_bytes_children);
}
+ path = list_delete_last(path);
+
+ total_bytes_children += stat.totalspace;
+ values[7] = Int64GetDatum(total_bytes_children);
+ values[8] = Int64GetDatum(stat.nblocks);
+ values[9] = Int64GetDatum(stat.freespace);
+ values[10] = Int64GetDatum(stat.freechunks);
+ values[11] = Int64GetDatum(stat.totalspace - stat.freespace);
+
+ tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+
+ *total_bytes_inc_children += total_bytes_children;
}
/*
@@ -120,10 +143,14 @@ Datum
pg_get_backend_memory_contexts(PG_FUNCTION_ARGS)
{
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+ int context_id = 0;
+ List *path = NIL;
+ Size total_bytes_inc_children = 0;
InitMaterializedSRF(fcinfo, 0);
PutMemoryContextsStatsTupleStore(rsinfo->setResult, rsinfo->setDesc,
- TopMemoryContext, NULL, 0);
+ TopMemoryContext, NULL, 0, &context_id,
+ path, &total_bytes_inc_children);
return (Datum) 0;
}
@@ -193,3 +220,25 @@ pg_log_backend_memory_contexts(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(true);
}
+
+/*
+ * Convert a list of context ids to a int[] Datum
+ */
+static Datum
+convert_path_to_datum(List *path)
+{
+ Datum *datum_array;
+ int length;
+ ArrayType *result_array;
+
+ length = list_length(path);
+ datum_array = (Datum *) palloc(length * sizeof(Datum));
+ length = 0;
+ foreach_int(id, path)
+ {
+ datum_array[length++] = Int32GetDatum(id);
+ }
+ result_array = construct_array_builtin(datum_array, length, INT4OID);
+
+ return PointerGetDatum(result_array);
+}
\ No newline at end of file
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 58811a6530..3299a6cfef 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8263,9 +8263,9 @@
proname => 'pg_get_backend_memory_contexts', prorows => '100',
proretset => 't', provolatile => 'v', proparallel => 'r',
prorettype => 'record', proargtypes => '',
- proallargtypes => '{text,text,text,int4,int8,int8,int8,int8,int8}',
- proargmodes => '{o,o,o,o,o,o,o,o,o}',
- proargnames => '{name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes}',
+ proallargtypes => '{text,int4,text,text,int4,_int4,int8,int8,int8,int8,int8,int8}',
+ proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o}',
+ proargnames => '{name, context_id, ident, parent, level, path, total_bytes, total_bytes_including_children, total_nblocks, free_bytes, free_chunks, used_bytes}',
prosrc => 'pg_get_backend_memory_contexts' },
# logging memory contexts of the specified backend
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 55f2e95352..e183d1185e 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1304,15 +1304,18 @@ pg_available_extensions| SELECT e.name,
FROM (pg_available_extensions() e(name, default_version, comment)
LEFT JOIN pg_extension x ON ((e.name = x.extname)));
pg_backend_memory_contexts| SELECT name,
+ context_id,
ident,
parent,
level,
+ path,
total_bytes,
+ total_bytes_including_children,
total_nblocks,
free_bytes,
free_chunks,
used_bytes
- FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes);
+ FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, context_id, ident, parent, level, path, total_bytes, total_bytes_including_children, total_nblocks, free_bytes, free_chunks, used_bytes);
pg_config| SELECT name,
setting
FROM pg_config() pg_config(name, setting);
diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out
index 271313ebf8..ad45404735 100644
--- a/src/test/regress/expected/sysviews.out
+++ b/src/test/regress/expected/sysviews.out
@@ -20,7 +20,7 @@ select count(*) >= 0 as ok from pg_available_extensions;
(1 row)
-- The entire output of pg_backend_memory_contexts is not stable,
--- we test only the existence and basic condition of TopMemoryContext.
+-- we test the existence and basic condition of TopMemoryContext.
select name, ident, parent, level, total_bytes >= free_bytes
from pg_backend_memory_contexts where level = 0;
name | ident | parent | level | ?column?
@@ -28,6 +28,27 @@ select name, ident, parent, level, total_bytes >= free_bytes
TopMemoryContext | | | 0 | t
(1 row)
+-- Test whether there are contexts with CacheMemoryContext in their path.
+-- There should be multiple children of CacheMemoryContext.
+with contexts as (
+ select * from pg_backend_memory_contexts
+)
+select count(*) > 0
+from contexts
+where array[(select context_id from contexts where name = 'CacheMemoryContext')] <@ path;
+ ?column?
+----------
+ t
+(1 row)
+
+-- TopMemoryContext should have the largest total_bytes_including_children.
+select name from pg_backend_memory_contexts
+ order by total_bytes_including_children desc limit 1;
+ name
+------------------
+ TopMemoryContext
+(1 row)
+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
ok
diff --git a/src/test/regress/sql/sysviews.sql b/src/test/regress/sql/sysviews.sql
index 6b4e24601d..64ecbc30cd 100644
--- a/src/test/regress/sql/sysviews.sql
+++ b/src/test/regress/sql/sysviews.sql
@@ -13,10 +13,23 @@ select count(*) >= 0 as ok from pg_available_extension_versions;
select count(*) >= 0 as ok from pg_available_extensions;
-- The entire output of pg_backend_memory_contexts is not stable,
--- we test only the existence and basic condition of TopMemoryContext.
+-- we test the existence and basic condition of TopMemoryContext.
select name, ident, parent, level, total_bytes >= free_bytes
from pg_backend_memory_contexts where level = 0;
+-- Test whether there are contexts with CacheMemoryContext in their path.
+-- There should be multiple children of CacheMemoryContext.
+with contexts as (
+ select * from pg_backend_memory_contexts
+)
+select count(*) > 0
+from contexts
+where array[(select context_id from contexts where name = 'CacheMemoryContext')] <@ path;
+
+-- TopMemoryContext should have the largest total_bytes_including_children.
+select name from pg_backend_memory_contexts
+ order by total_bytes_including_children desc limit 1;
+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
--
2.34.1
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
2023-10-23 12:02 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-12-04 04:43 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-03 11:40 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-10 06:37 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-16 09:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
@ 2024-01-19 08:41 ` torikoshia <[email protected]>
2024-02-14 07:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Michael Paquier <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: torikoshia @ 2024-01-19 08:41 UTC (permalink / raw)
To: Melih Mutlu <[email protected]>; +Cc: Andres Freund <[email protected]>; Stephen Frost <[email protected]>; PostgreSQL Hackers <[email protected]>
On 2024-01-16 18:41, Melih Mutlu wrote:
> Hi,
>
> Thanks for reviewing.
>
> torikoshia <[email protected]>, 10 Oca 2024 Çar, 09:37
> tarihinde şunu yazdı:
>
>>> + <row>
>>> + <entry role="catalog_table_entry"><para
>>> role="column_definition">
>>> + <structfield>context_id</structfield> <type>int4</type>
>>> + </para>
>>> + <para>
>>> + Current context id. Note that the context id is a
>> temporary id
>>> and may
>>> + change in each invocation
>>> + </para></entry>
>>> + </row>
>>> +
>>> + <row>
>>> + <entry role="catalog_table_entry"><para
>>> role="column_definition">
>>> + <structfield>path</structfield> <type>int4[]</type>
>>> + </para>
>>> + <para>
>>> + Path to reach the current context from TopMemoryContext.
>>> Context ids in
>>> + this list represents all parents of the current context.
>> This
>>> can be
>>> + used to build the parent and child relation
>>> + </para></entry>
>>> + </row>
>>> +
>>> + <row>
>>> + <entry role="catalog_table_entry"><para
>>> role="column_definition">
>>> + <structfield>total_bytes_including_children</structfield>
>>> <type>int8</type>
>>> + </para>
>>> + <para>
>>> + Total bytes allocated for this memory context including
>> its
>>> children
>>> + </para></entry>
>>> + </row>
>>
>> These columns are currently added to the bottom of the table, but it
>> may
>> be better to put semantically similar items close together and
>> change
>> the insertion position with reference to other system views. For
>> example,
>>
>> - In pg_group and pg_user, 'id' is placed on the line following
>> 'name',
>> so 'context_id' be placed on the line following 'name'
>> - 'path' is similar with 'parent' and 'level' in that these are
>> information about the location of the context, 'path' be placed to
>> next
>> to them.
>>
>> If we do this, orders of columns in the system view should be the
>> same,
>> I think.
>
> I've done what you suggested. Also moved
> "total_bytes_including_children" right after "total_bytes".
>
>> 14dd0f27d have introduced new macro foreach_int.
>> It seems to be able to make the code a bit simpler and the commit
>> log
>> says this macro is primarily intended for use in new code. For
>> example:
>
> Makes sense. Done.
Thanks for updating the patch!
> + Current context id. Note that the context id is a temporary id
> and may
> + change in each invocation
> + </para></entry>
> + </row>
It clearly states that the context id is temporary, but I am a little
concerned about users who write queries that refer to this view multiple
times without using CTE.
If you agree, how about adding some description like below you mentioned
before?
> We still need to use cte since ids are not persisted and might change
> in
> each run of pg_backend_memory_contexts. Materializing the result can
> prevent any inconsistencies due to id change. Also it can be even good
> for
> performance reasons as well.
We already have additional description below the table which explains
each column of the system view. For example pg_locks:
https://www.postgresql.org/docs/devel/view-pg-locks.html
Also giving an example query something like this might be useful.
-- show all the parent context names of ExecutorState
with contexts as (
select * from pg_backend_memory_contexts
)
select name from contexts where array[context_id] <@ (select path from
contexts where name = 'ExecutorState');
--
Regards,
--
Atsushi Torikoshi
NTT DATA Group Corporation
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
2023-10-23 12:02 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-12-04 04:43 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-03 11:40 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-10 06:37 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-16 09:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-19 08:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
@ 2024-02-14 07:23 ` Michael Paquier <[email protected]>
2024-04-03 13:20 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Michael Paquier @ 2024-02-14 07:23 UTC (permalink / raw)
To: torikoshia <[email protected]>; +Cc: Melih Mutlu <[email protected]>; Andres Freund <[email protected]>; Stephen Frost <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, Jan 19, 2024 at 05:41:45PM +0900, torikoshia wrote:
> We already have additional description below the table which explains each
> column of the system view. For example pg_locks:
> https://www.postgresql.org/docs/devel/view-pg-locks.html
I was reading the patch, and using int[] as a representation of the
path of context IDs up to the top-most parent looks a bit strange to
me, with the relationship between each parent -> child being
preserved, visibly, based on the order of the elements in this array
made of temporary IDs compiled on-the-fly during the function
execution. Am I the only one finding that a bit strange? Could it be
better to use a different data type for this path and perhaps switch
to the names of the contexts involved?
It is possible to retrieve this information some WITH RECURSIVE as
well, as mentioned upthread. Perhaps we could consider documenting
these tricks?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
2023-10-23 12:02 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-12-04 04:43 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-03 11:40 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-10 06:37 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-16 09:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-19 08:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-02-14 07:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Michael Paquier <[email protected]>
@ 2024-04-03 13:20 ` Melih Mutlu <[email protected]>
2024-04-03 23:34 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Michael Paquier <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Melih Mutlu @ 2024-04-03 13:20 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: torikoshia <[email protected]>; Andres Freund <[email protected]>; Stephen Frost <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
Michael Paquier <[email protected]>, 14 Şub 2024 Çar, 10:23 tarihinde
şunu yazdı:
> On Fri, Jan 19, 2024 at 05:41:45PM +0900, torikoshia wrote:
> > We already have additional description below the table which explains
> each
> > column of the system view. For example pg_locks:
> > https://www.postgresql.org/docs/devel/view-pg-locks.html
>
> I was reading the patch, and using int[] as a representation of the
> path of context IDs up to the top-most parent looks a bit strange to
> me, with the relationship between each parent -> child being
> preserved, visibly, based on the order of the elements in this array
> made of temporary IDs compiled on-the-fly during the function
> execution. Am I the only one finding that a bit strange? Could it be
> better to use a different data type for this path and perhaps switch
> to the names of the contexts involved?
>
Do you find having the path column strange all together? Or only using
temporary IDs to generate that column? The reason why I avoid using context
names is because there can be multiple contexts with the same name. This
makes it difficult to figure out which context, among those with that
particular name, is actually included in the path. I couldn't find any
other information that is unique to each context.
Thanks,
--
Melih Mutlu
Microsoft
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
2023-10-23 12:02 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-12-04 04:43 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-03 11:40 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-10 06:37 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-16 09:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-19 08:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-02-14 07:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Michael Paquier <[email protected]>
2024-04-03 13:20 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
@ 2024-04-03 23:34 ` Michael Paquier <[email protected]>
2024-04-04 01:44 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() David Rowley <[email protected]>
2024-07-10 21:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Robert Haas <[email protected]>
0 siblings, 2 replies; 19+ messages in thread
From: Michael Paquier @ 2024-04-03 23:34 UTC (permalink / raw)
To: Melih Mutlu <[email protected]>; +Cc: torikoshia <[email protected]>; Andres Freund <[email protected]>; Stephen Frost <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Apr 03, 2024 at 04:20:39PM +0300, Melih Mutlu wrote:
> Michael Paquier <[email protected]>, 14 Şub 2024 Çar, 10:23 tarihinde
> şunu yazdı:
>> I was reading the patch, and using int[] as a representation of the
>> path of context IDs up to the top-most parent looks a bit strange to
>> me, with the relationship between each parent -> child being
>> preserved, visibly, based on the order of the elements in this array
>> made of temporary IDs compiled on-the-fly during the function
>> execution. Am I the only one finding that a bit strange? Could it be
>> better to use a different data type for this path and perhaps switch
>> to the names of the contexts involved?
>
> Do you find having the path column strange all together? Or only using
> temporary IDs to generate that column? The reason why I avoid using context
> names is because there can be multiple contexts with the same name. This
> makes it difficult to figure out which context, among those with that
> particular name, is actually included in the path. I couldn't find any
> other information that is unique to each context.
I've been re-reading the patch again to remember what this is about,
and I'm OK with having this "path" column in the catalog. However,
I'm somewhat confused by the choice of having a temporary number that
shows up in the catalog representation, because this may not be
constant across multiple calls so this still requires a follow-up
temporary ID <-> name mapping in any SQL querying this catalog. A
second thing is that array does not show the hierarchy of the path;
the patch relies on the order of the elements in the output array
instead.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
2023-10-23 12:02 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-12-04 04:43 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-03 11:40 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-10 06:37 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-16 09:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-19 08:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-02-14 07:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Michael Paquier <[email protected]>
2024-04-03 13:20 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-04-03 23:34 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Michael Paquier <[email protected]>
@ 2024-04-04 01:44 ` David Rowley <[email protected]>
2024-07-02 13:08 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
1 sibling, 1 reply; 19+ messages in thread
From: David Rowley @ 2024-04-04 01:44 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Melih Mutlu <[email protected]>; torikoshia <[email protected]>; Andres Freund <[email protected]>; Stephen Frost <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, 4 Apr 2024 at 12:34, Michael Paquier <[email protected]> wrote:
> I've been re-reading the patch again to remember what this is about,
> and I'm OK with having this "path" column in the catalog. However,
> I'm somewhat confused by the choice of having a temporary number that
> shows up in the catalog representation, because this may not be
> constant across multiple calls so this still requires a follow-up
> temporary ID <-> name mapping in any SQL querying this catalog. A
> second thing is that array does not show the hierarchy of the path;
> the patch relies on the order of the elements in the output array
> instead.
My view on this is that there are a couple of things with the patch
which could be considered separately:
1. Should we have a context_id in the view?
2. Should we also have an array of all parents?
My view is that we really need #1 as there's currently no reliable way
to determine a context's parent as the names are not unique. I do
see that Melih has mentioned this is temporary in:
+ <para>
+ Current context id. Note that the context id is a temporary id and may
+ change in each invocation
+ </para></entry>
For #2, I'm a bit less sure about this. I know Andres would like to
see this array added, but equally WITH RECURSIVE would work. Does the
array of parents completely eliminate the need for recursive queries?
I think the array works for anything that requires all parents or some
fixed (would be) recursive level, but there might be some other
condition to stop recursion other than the recursion level that
someone needs to do. What I'm trying to get at is; do we need to
document the WITH RECURSIVE stuff anyway? and if we do, is it still
worth having the parents array?
David
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
2023-10-23 12:02 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-12-04 04:43 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-03 11:40 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-10 06:37 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-16 09:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-19 08:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-02-14 07:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Michael Paquier <[email protected]>
2024-04-03 13:20 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-04-03 23:34 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Michael Paquier <[email protected]>
2024-04-04 01:44 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() David Rowley <[email protected]>
@ 2024-07-02 13:08 ` Melih Mutlu <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: Melih Mutlu @ 2024-07-02 13:08 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Michael Paquier <[email protected]>; torikoshia <[email protected]>; Andres Freund <[email protected]>; Stephen Frost <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi hackers,
David Rowley <[email protected]>, 4 Nis 2024 Per, 04:44 tarihinde şunu
yazdı:
> My view on this is that there are a couple of things with the patch
> which could be considered separately:
>
> 1. Should we have a context_id in the view?
> 2. Should we also have an array of all parents?
>
I discussed the above questions with David off-list, and decided to make
some changes in the patch as a result. I'd appreciate any input.
First of all, I agree that previous versions of the patch could make things
seem a bit more complicated than they should be, by having three new
columns (context_id, path, total_bytes_including_children). Especially when
we could already get the same result with several different ways (e.g.
writing a recursive query, using the patch column, and the
total_bytes_including_children column by itself help to know total used
bytes by a contexts and all of its children)
I believe that we really need to have context IDs as it's the only unique
way to identify a context. And I'm for having a parents array as it makes
things easier and demonstrates the parent/child relation explicitly. One
idea to simplify this patch a bit is adding the ID of a context into its
own path and removing the context_id column. As those IDs are temporary, I
don't think they would be useful other than using them to find some kind of
relation by looking into path values of some other rows. So maybe not
having a separate column for IDs but only having the path can help with the
confusion which this patch might introduce. The last element of the patch
would simply be the ID of that particular context.
One nice thing which David pointed out about paths is that level
information can become useful in those arrays. Level can represent the
position of a context in the path arrays of its child contexts. For
example; TopMemoryContext will always be the first element in all paths as
it's the top-most parent, it's also the only context with level 0. So this
relation between levels and indexes in path arrays can be somewhat useful
to link this array with the overall hierarchy of memory contexts.
An example query to get total used bytes including children by using level
info would look like:
WITH contexts AS (
SELECT * FROM pg_backend_memory_contexts
)
SELECT sum(total_bytes)
FROM contexts
WHERE path[( SELECT level+1 FROM contexts WHERE name =
'CacheMemoryContext')] =
(SELECT path[level+1] FROM contexts WHERE name = 'CacheMemoryContext');
Lastly, I created a separate patch to add total_bytes_including_children
columns. I understand that sum of total_bytes of a context and its children
will likely be one of the frequently used cases, not everyone may agree
with having an _including_children column for only total_bytes. I'm open to
hear more opinions on this.
Best Regards,
--
Melih Mutlu
Microsoft
Attachments:
[application/octet-stream] v6-0002-Add-total_bytes_including_children-column.patch (7.6K, ../../CAGPVpCSTR8c+koRzoNnxEvcOgVH2yLDMcnH67z8a5ehKfYiheg@mail.gmail.com/3-v6-0002-Add-total_bytes_including_children-column.patch)
download | inline diff:
From 39e4bb039900c270b42475648c8bb0ae64a3f14c Mon Sep 17 00:00:00 2001
From: Melih Mutlu <[email protected]>
Date: Mon, 1 Jul 2024 16:06:03 +0300
Subject: [PATCH v6 2/2] Add total_bytes_including_children column
Add a new column total_bytes_including_children into
pg_backend_memory_contexts.
---
doc/src/sgml/system-views.sgml | 9 +++++++++
src/backend/utils/adt/mcxtfuncs.c | 28 +++++++++++++++++---------
src/include/catalog/pg_proc.dat | 6 +++---
src/test/regress/expected/rules.out | 3 ++-
src/test/regress/expected/sysviews.out | 14 +++++++++++++
src/test/regress/sql/sysviews.sql | 10 +++++++++
6 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index f818aba974..ad2cb71a88 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -530,6 +530,15 @@
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>total_bytes_including_children</structfield> <type>int8</type>
+ </para>
+ <para>
+ Total bytes allocated for this memory context including its children
+ </para></entry>
+ </row>
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>total_nblocks</structfield> <type>int8</type>
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index 82b863341c..ca1eb0840e 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -38,9 +38,9 @@ static void
PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
TupleDesc tupdesc, MemoryContext context,
const char *parent, int level, int *context_id,
- List *path)
+ List *path, Size *total_bytes_inc_children)
{
-#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 11
+#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 12
Datum values[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
bool nulls[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
@@ -50,6 +50,7 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
const char *ident;
const char *type;
int current_context_id = (*context_id)++;
+ Size total_bytes_children = 0;
Assert(MemoryContextIsValid(context));
@@ -127,20 +128,26 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
path = lappend_int(path, current_context_id);
values[5] = convert_path_to_datum(path);
- values[6] = Int64GetDatum(stat.totalspace);
- values[7] = Int64GetDatum(stat.nblocks);
- values[8] = Int64GetDatum(stat.freespace);
- values[9] = Int64GetDatum(stat.freechunks);
- values[10] = Int64GetDatum(stat.totalspace - stat.freespace);
-
+ total_bytes_children += stat.totalspace;
for (child = context->firstchild; child != NULL; child = child->nextchild)
{
PutMemoryContextsStatsTupleStore(tupstore, tupdesc, child, name,
- level+1, context_id, path);
+ level+1, context_id, path,
+ &total_bytes_children);
}
path = list_delete_last(path);
+
+ values[6] = Int64GetDatum(stat.totalspace);
+ values[7] = Int64GetDatum(total_bytes_children);
+ values[8] = Int64GetDatum(stat.nblocks);
+ values[9] = Int64GetDatum(stat.freespace);
+ values[10] = Int64GetDatum(stat.freechunks);
+ values[11] = Int64GetDatum(stat.totalspace - stat.freespace);
+
tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+
+ *total_bytes_inc_children += total_bytes_children;
}
/*
@@ -153,11 +160,12 @@ pg_get_backend_memory_contexts(PG_FUNCTION_ARGS)
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
int context_id = 0;
List *path = NIL;
+ Size total_bytes_inc_children = 0;
InitMaterializedSRF(fcinfo, 0);
PutMemoryContextsStatsTupleStore(rsinfo->setResult, rsinfo->setDesc,
TopMemoryContext, NULL, 0, &context_id,
- path);
+ path, &total_bytes_inc_children);
return (Datum) 0;
}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 202a0b7567..be31190f62 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8279,9 +8279,9 @@
proname => 'pg_get_backend_memory_contexts', prorows => '100',
proretset => 't', provolatile => 'v', proparallel => 'r',
prorettype => 'record', proargtypes => '',
- proallargtypes => '{text,text,text,text,int4,_int4,int8,int8,int8,int8,int8}',
- proargmodes => '{o,o,o,o,o,o,o,o,o,o,o}',
- proargnames => '{name, ident, parent, type, level, path, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes}',
+ proallargtypes => '{text,text,text,text,int4,_int4,int8,int8,int8,int8,int8,int8}',
+ proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o}',
+ proargnames => '{name, ident, parent, type, level, path, total_bytes, total_bytes_including_children, total_nblocks, free_bytes, free_chunks, used_bytes}',
prosrc => 'pg_get_backend_memory_contexts' },
# logging memory contexts of the specified backend
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 5201280669..9318cb5212 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1310,11 +1310,12 @@ pg_backend_memory_contexts| SELECT name,
level,
path,
total_bytes,
+ total_bytes_including_children,
total_nblocks,
free_bytes,
free_chunks,
used_bytes
- FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, type, level, path, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes);
+ FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, type, level, path, total_bytes, total_bytes_including_children, total_nblocks, free_bytes, free_chunks, used_bytes);
pg_config| SELECT name,
setting
FROM pg_config() pg_config(name, setting);
diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out
index b27b7dca4b..fa64b6de9b 100644
--- a/src/test/regress/expected/sysviews.out
+++ b/src/test/regress/expected/sysviews.out
@@ -64,6 +64,20 @@ where array[(select path[level+1] from contexts where name = 'CacheMemoryContext
t
(1 row)
+-- Test whether total_bytes_including_children really sums up to the total
+-- bytes used by all child contexts of CacheMemoryContext.
+with contexts as (
+ select * from pg_backend_memory_contexts
+)
+select sum(total_bytes) =
+ (select total_bytes_including_children from contexts where name = 'CacheMemoryContext')
+from contexts
+where array[(select path[level+1] from contexts where name = 'CacheMemoryContext')] <@ path;
+ ?column?
+----------
+ t
+(1 row)
+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
ok
diff --git a/src/test/regress/sql/sysviews.sql b/src/test/regress/sql/sysviews.sql
index 0953d3e2c7..91da7eb5bb 100644
--- a/src/test/regress/sql/sysviews.sql
+++ b/src/test/regress/sql/sysviews.sql
@@ -41,6 +41,16 @@ select count(*) > 0
from contexts
where array[(select path[level+1] from contexts where name = 'CacheMemoryContext')] <@ path;
+-- Test whether total_bytes_including_children really sums up to the total
+-- bytes used by all child contexts of CacheMemoryContext.
+with contexts as (
+ select * from pg_backend_memory_contexts
+)
+select sum(total_bytes) =
+ (select total_bytes_including_children from contexts where name = 'CacheMemoryContext')
+from contexts
+where array[(select path[level+1] from contexts where name = 'CacheMemoryContext')] <@ path;
+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
--
2.34.1
[application/octet-stream] v6-0001-Add-path-column-into-pg_backend_memory_contexts.patch (9.8K, ../../CAGPVpCSTR8c+koRzoNnxEvcOgVH2yLDMcnH67z8a5ehKfYiheg@mail.gmail.com/4-v6-0001-Add-path-column-into-pg_backend_memory_contexts.patch)
download | inline diff:
From 54086bd9923322459e1484a45b30c9bf934ae7ac Mon Sep 17 00:00:00 2001
From: Melih Mutlu <[email protected]>
Date: Mon, 1 Jul 2024 13:00:51 +0300
Subject: [PATCH v6 1/2] Add path column into pg_backend_memory_contexts
This patch adds a new column into the tuples returned by
pg_get_backend_memory_contexts() to specify the parent/child relation
between memory contexts and the path from root to current context.
Context names cannot be relied on since they're not unique. Therefore,
unique IDs are needed for each context. Those new IDs are assigned during
pg_get_backend_memory_contexts() call and not stored anywhere. So they
may change in each pg_get_backend_memory_contexts() call and shouldn't be
used across different pg_get_backend_memory_contexts() calls.
---
doc/src/sgml/system-views.sgml | 32 ++++++++++++++
src/backend/utils/adt/mcxtfuncs.c | 58 +++++++++++++++++++++-----
src/include/catalog/pg_proc.dat | 6 +--
src/test/regress/expected/rules.out | 3 +-
src/test/regress/expected/sysviews.out | 13 ++++++
src/test/regress/sql/sysviews.sql | 9 ++++
6 files changed, 106 insertions(+), 15 deletions(-)
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index bdc34cf94e..f818aba974 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -508,6 +508,19 @@
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>path</structfield> <type>int4[]</type>
+ </para>
+ <para>
+ Path that includes all contexts from TopMemoryContext to the current
+ context. Context IDs in this list represents all parents of a context,
+ and last element in the list refers to the context itself. This can be
+ used to build the parent and child relation. Note that IDs used in path
+ are transient and may change in each execution
+ </para></entry>
+ </row>
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>total_bytes</structfield> <type>int8</type>
@@ -561,6 +574,25 @@
read only by superusers or roles with the privileges of the
<literal>pg_read_all_stats</literal> role.
</para>
+
+ <para>
+ The <structfield>path</structfield> column can be useful to build
+ parent/child relation between memory contexts. For example, the following
+ query calculates the total number of bytes used by a memory context and its
+ child contexts:
+<programlisting>
+WITH memory_contexts AS (
+ SELECT *
+ FROM pg_backend_memory_contexts
+)
+SELECT SUM(total_bytes)
+FROM memory_contexts
+WHERE ARRAY[(SELECT path[array_length(path, 1)] FROM memory_contexts WHERE name = 'CacheMemoryContext')] <@ path;
+</programlisting>
+ Also, <link linkend="queries-with">Common Table Expressions</link> can be
+ useful while working with context IDs as these IDs are temporary and may
+ change in each invocation.
+ </para>
</sect1>
<sect1 id="view-pg-config">
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index 1085941484..82b863341c 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -19,6 +19,7 @@
#include "mb/pg_wchar.h"
#include "storage/proc.h"
#include "storage/procarray.h"
+#include "utils/array.h"
#include "utils/builtins.h"
/* ----------
@@ -27,6 +28,8 @@
*/
#define MEMORY_CONTEXT_IDENT_DISPLAY_SIZE 1024
+static Datum convert_path_to_datum(List *path);
+
/*
* PutMemoryContextsStatsTupleStore
* One recursion level for pg_get_backend_memory_contexts.
@@ -34,9 +37,10 @@
static void
PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
TupleDesc tupdesc, MemoryContext context,
- const char *parent, int level)
+ const char *parent, int level, int *context_id,
+ List *path)
{
-#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 10
+#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 11
Datum values[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
bool nulls[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS];
@@ -45,6 +49,7 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
const char *name;
const char *ident;
const char *type;
+ int current_context_id = (*context_id)++;
Assert(MemoryContextIsValid(context));
@@ -118,18 +123,24 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore,
values[3] = CStringGetTextDatum(type);
values[4] = Int32GetDatum(level);
- values[5] = Int64GetDatum(stat.totalspace);
- values[6] = Int64GetDatum(stat.nblocks);
- values[7] = Int64GetDatum(stat.freespace);
- values[8] = Int64GetDatum(stat.freechunks);
- values[9] = Int64GetDatum(stat.totalspace - stat.freespace);
- tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+
+ path = lappend_int(path, current_context_id);
+ values[5] = convert_path_to_datum(path);
+
+ values[6] = Int64GetDatum(stat.totalspace);
+ values[7] = Int64GetDatum(stat.nblocks);
+ values[8] = Int64GetDatum(stat.freespace);
+ values[9] = Int64GetDatum(stat.freechunks);
+ values[10] = Int64GetDatum(stat.totalspace - stat.freespace);
for (child = context->firstchild; child != NULL; child = child->nextchild)
{
- PutMemoryContextsStatsTupleStore(tupstore, tupdesc,
- child, name, level + 1);
+ PutMemoryContextsStatsTupleStore(tupstore, tupdesc, child, name,
+ level+1, context_id, path);
}
+ path = list_delete_last(path);
+
+ tuplestore_putvalues(tupstore, tupdesc, values, nulls);
}
/*
@@ -140,10 +151,13 @@ Datum
pg_get_backend_memory_contexts(PG_FUNCTION_ARGS)
{
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+ int context_id = 0;
+ List *path = NIL;
InitMaterializedSRF(fcinfo, 0);
PutMemoryContextsStatsTupleStore(rsinfo->setResult, rsinfo->setDesc,
- TopMemoryContext, NULL, 0);
+ TopMemoryContext, NULL, 0, &context_id,
+ path);
return (Datum) 0;
}
@@ -206,3 +220,25 @@ pg_log_backend_memory_contexts(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(true);
}
+
+/*
+ * Convert a list of context ids to a int[] Datum
+ */
+static Datum
+convert_path_to_datum(List *path)
+{
+ Datum *datum_array;
+ int length;
+ ArrayType *result_array;
+
+ length = list_length(path);
+ datum_array = (Datum *) palloc(length * sizeof(Datum));
+ length = 0;
+ foreach_int(id, path)
+ {
+ datum_array[length++] = Int32GetDatum(id);
+ }
+ result_array = construct_array_builtin(datum_array, length, INT4OID);
+
+ return PointerGetDatum(result_array);
+}
\ No newline at end of file
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index d4ac578ae6..202a0b7567 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8279,9 +8279,9 @@
proname => 'pg_get_backend_memory_contexts', prorows => '100',
proretset => 't', provolatile => 'v', proparallel => 'r',
prorettype => 'record', proargtypes => '',
- proallargtypes => '{text,text,text,text,int4,int8,int8,int8,int8,int8}',
- proargmodes => '{o,o,o,o,o,o,o,o,o,o}',
- proargnames => '{name, ident, parent, type, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes}',
+ proallargtypes => '{text,text,text,text,int4,_int4,int8,int8,int8,int8,int8}',
+ proargmodes => '{o,o,o,o,o,o,o,o,o,o,o}',
+ proargnames => '{name, ident, parent, type, level, path, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes}',
prosrc => 'pg_get_backend_memory_contexts' },
# logging memory contexts of the specified backend
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 4c789279e5..5201280669 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1308,12 +1308,13 @@ pg_backend_memory_contexts| SELECT name,
parent,
type,
level,
+ path,
total_bytes,
total_nblocks,
free_bytes,
free_chunks,
used_bytes
- FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, type, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes);
+ FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, type, level, path, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes);
pg_config| SELECT name,
setting
FROM pg_config() pg_config(name, setting);
diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out
index 729620de13..b27b7dca4b 100644
--- a/src/test/regress/expected/sysviews.out
+++ b/src/test/regress/expected/sysviews.out
@@ -51,6 +51,19 @@ from pg_backend_memory_contexts where name = 'Caller tuples';
(1 row)
rollback;
+-- Test whether there are contexts with CacheMemoryContext in their path.
+-- There should be multiple children of CacheMemoryContext.
+with contexts as (
+ select * from pg_backend_memory_contexts
+)
+select count(*) > 0
+from contexts
+where array[(select path[level+1] from contexts where name = 'CacheMemoryContext')] <@ path;
+ ?column?
+----------
+ t
+(1 row)
+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
ok
diff --git a/src/test/regress/sql/sysviews.sql b/src/test/regress/sql/sysviews.sql
index 7edac2fde1..0953d3e2c7 100644
--- a/src/test/regress/sql/sysviews.sql
+++ b/src/test/regress/sql/sysviews.sql
@@ -32,6 +32,15 @@ select type, name, parent, total_bytes > 0, total_nblocks, free_bytes > 0, free_
from pg_backend_memory_contexts where name = 'Caller tuples';
rollback;
+-- Test whether there are contexts with CacheMemoryContext in their path.
+-- There should be multiple children of CacheMemoryContext.
+with contexts as (
+ select * from pg_backend_memory_contexts
+)
+select count(*) > 0
+from contexts
+where array[(select path[level+1] from contexts where name = 'CacheMemoryContext')] <@ path;
+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
--
2.34.1
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
2023-10-23 12:02 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-12-04 04:43 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-03 11:40 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-10 06:37 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-01-16 09:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-01-19 08:41 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() torikoshia <[email protected]>
2024-02-14 07:23 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Michael Paquier <[email protected]>
2024-04-03 13:20 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2024-04-03 23:34 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Michael Paquier <[email protected]>
@ 2024-07-10 21:16 ` Robert Haas <[email protected]>
1 sibling, 0 replies; 19+ messages in thread
From: Robert Haas @ 2024-07-10 21:16 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Melih Mutlu <[email protected]>; torikoshia <[email protected]>; Andres Freund <[email protected]>; Stephen Frost <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Apr 3, 2024 at 7:34 PM Michael Paquier <[email protected]> wrote:
> I've been re-reading the patch again to remember what this is about,
> and I'm OK with having this "path" column in the catalog. However,
> I'm somewhat confused by the choice of having a temporary number that
> shows up in the catalog representation, because this may not be
> constant across multiple calls so this still requires a follow-up
> temporary ID <-> name mapping in any SQL querying this catalog. A
> second thing is that array does not show the hierarchy of the path;
> the patch relies on the order of the elements in the output array
> instead.
This complaint doesn't seem reasonable to me. The point of the path,
as I understand it, is to allow the caller to make sense of the
results of a single call, which is otherwise impossible. Stability
across multiple calls would be much more difficult, particularly
because we have no unique, long-lived identifier for memory contexts,
except perhaps the address of the context. Exposing the pointer
address of the memory contexts to clients would be an extremely bad
idea from a security point of view -- and it also seems unnecessary,
because the point of this function is to get a clear snapshot of
memory usage at a particular moment, not to track changes in usage by
the same contexts over time. You could still build the latter on top
of this if you wanted to do that, but I don't think most people would,
and I don't think the transient path IDs make it any more difficult.
I feel like Melih has chosen a simple and natural representation and I
would have done pretty much the same thing. And AFAICS there's no
reasonable alternative design.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
@ 2023-10-18 19:53 ` Stephen Frost <[email protected]>
2023-10-19 01:17 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
1 sibling, 1 reply; 19+ messages in thread
From: Stephen Frost @ 2023-10-18 19:53 UTC (permalink / raw)
To: Melih Mutlu <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Greetings,
* Melih Mutlu ([email protected]) wrote:
> Melih Mutlu <[email protected]>, 16 Haz 2023 Cum, 17:03 tarihinde şunu
> yazdı:
>
> > With this change, here's a query to find how much space used by each
> > context including its children:
> >
> > > WITH RECURSIVE cte AS (
> > > SELECT id, total_bytes, id as root, name as root_name
> > > FROM memory_contexts
> > > UNION ALL
> > > SELECT r.id, r.total_bytes, cte.root, cte.root_name
> > > FROM memory_contexts r
> > > INNER JOIN cte ON r.parent_id = cte.id
> > > ),
> > > memory_contexts AS (
> > > SELECT * FROM pg_backend_memory_contexts
> > > )
> > > SELECT root as id, root_name as name, sum(total_bytes)
> > > FROM cte
> > > GROUP BY root, root_name
> > > ORDER BY sum DESC;
>
> Given that the above query to get total bytes including all children is
> still a complex one, I decided to add an additional info in
> pg_backend_memory_contexts.
> The new "path" field displays an integer array that consists of ids of all
> parents for the current context. This way it's easier to tell whether a
> context is a child of another context, and we don't need to use recursive
> queries to get this info.
Nice, this does seem quite useful.
> Here how pg_backend_memory_contexts would look like with this patch:
>
> postgres=# SELECT name, id, parent, parent_id, path
> FROM pg_backend_memory_contexts
> ORDER BY total_bytes DESC LIMIT 10;
> name | id | parent | parent_id | path
> -------------------------+-----+------------------+-----------+--------------
> CacheMemoryContext | 27 | TopMemoryContext | 0 | {0}
> Timezones | 124 | TopMemoryContext | 0 | {0}
> TopMemoryContext | 0 | | |
> MessageContext | 8 | TopMemoryContext | 0 | {0}
> WAL record construction | 118 | TopMemoryContext | 0 | {0}
> ExecutorState | 18 | PortalContext | 17 | {0,16,17}
> TupleSort main | 19 | ExecutorState | 18 | {0,16,17,18}
> TransactionAbortContext | 14 | TopMemoryContext | 0 | {0}
> smgr relation table | 10 | TopMemoryContext | 0 | {0}
> GUC hash table | 123 | GUCMemoryContext | 122 | {0,122}
> (10 rows)
>
> An example query to calculate the total_bytes including its children for a
> context (say CacheMemoryContext) would look like this:
>
> WITH contexts AS (
> SELECT * FROM pg_backend_memory_contexts
> )
> SELECT sum(total_bytes)
> FROM contexts
> WHERE ARRAY[(SELECT id FROM contexts WHERE name = 'CacheMemoryContext')] <@
> path;
I wonder if we should perhaps just include
"total_bytes_including_children" as another column? Certainly seems
like a very useful thing that folks would like to see. We could do that
either with C, or even something as simple as changing the view to do
something like:
WITH contexts AS MATERIALIZED (
SELECT * FROM pg_get_backend_memory_contexts()
)
SELECT
*,
coalesce
(
(
(SELECT sum(total_bytes) FROM contexts WHERE ARRAY[a.id] <@ path)
+ total_bytes
),
total_bytes
) AS total_bytes_including_children
FROM contexts a;
> We still need to use cte since ids are not persisted and might change in
> each run of pg_backend_memory_contexts. Materializing the result can
> prevent any inconsistencies due to id change. Also it can be even good for
> performance reasons as well.
I don't think we really want this to be materialized, do we? Where this
is particularly interesting is when it's being dumped to the log ( ...
though I wish we could do better than that and hope we do in the future)
while something is ongoing in a given backend and if we do that a few
times we are able to see what's changing in terms of allocations,
whereas if we materialized it (when? transaction start? first time
it's asked for?) then we'd only ever get the one view from whenever the
snapshot was taken.
> Any thoughts?
Generally +1 from me for working on improving this.
Thanks!
Stephen
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-18 19:53 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Stephen Frost <[email protected]>
@ 2023-10-19 01:17 ` Andres Freund <[email protected]>
2023-10-19 22:01 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Stephen Frost <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Andres Freund @ 2023-10-19 01:17 UTC (permalink / raw)
To: Stephen Frost <[email protected]>; +Cc: Melih Mutlu <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On 2023-10-18 15:53:30 -0400, Stephen Frost wrote:
> > Here how pg_backend_memory_contexts would look like with this patch:
> >
> > postgres=# SELECT name, id, parent, parent_id, path
> > FROM pg_backend_memory_contexts
> > ORDER BY total_bytes DESC LIMIT 10;
> > name | id | parent | parent_id | path
> > -------------------------+-----+------------------+-----------+--------------
> > CacheMemoryContext | 27 | TopMemoryContext | 0 | {0}
> > Timezones | 124 | TopMemoryContext | 0 | {0}
> > TopMemoryContext | 0 | | |
> > MessageContext | 8 | TopMemoryContext | 0 | {0}
> > WAL record construction | 118 | TopMemoryContext | 0 | {0}
> > ExecutorState | 18 | PortalContext | 17 | {0,16,17}
> > TupleSort main | 19 | ExecutorState | 18 | {0,16,17,18}
> > TransactionAbortContext | 14 | TopMemoryContext | 0 | {0}
> > smgr relation table | 10 | TopMemoryContext | 0 | {0}
> > GUC hash table | 123 | GUCMemoryContext | 122 | {0,122}
> > (10 rows)
> >
> > An example query to calculate the total_bytes including its children for a
> > context (say CacheMemoryContext) would look like this:
> >
> > WITH contexts AS (
> > SELECT * FROM pg_backend_memory_contexts
> > )
> > SELECT sum(total_bytes)
> > FROM contexts
> > WHERE ARRAY[(SELECT id FROM contexts WHERE name = 'CacheMemoryContext')] <@
> > path;
>
> I wonder if we should perhaps just include
> "total_bytes_including_children" as another column? Certainly seems
> like a very useful thing that folks would like to see.
The "issue" is where to stop - should we also add that for some of the other
columns? They are a bit less important, but not that much.
> > We still need to use cte since ids are not persisted and might change in
> > each run of pg_backend_memory_contexts. Materializing the result can
> > prevent any inconsistencies due to id change. Also it can be even good for
> > performance reasons as well.
>
> I don't think we really want this to be materialized, do we? Where this
> is particularly interesting is when it's being dumped to the log ( ...
> though I wish we could do better than that and hope we do in the future)
> while something is ongoing in a given backend and if we do that a few
> times we are able to see what's changing in terms of allocations,
> whereas if we materialized it (when? transaction start? first time
> it's asked for?) then we'd only ever get the one view from whenever the
> snapshot was taken.
I think the comment was just about the need to use a CTE, because self-joining
with divergent versions of pg_backend_memory_contexts would not always work
out well.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Parent/child context relation in pg_get_backend_memory_contexts()
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-10-18 19:53 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Stephen Frost <[email protected]>
2023-10-19 01:17 ` Re: Parent/child context relation in pg_get_backend_memory_contexts() Andres Freund <[email protected]>
@ 2023-10-19 22:01 ` Stephen Frost <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: Stephen Frost @ 2023-10-19 22:01 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Melih Mutlu <[email protected]>; PostgreSQL Hackers <[email protected]>
Greetings,
* Andres Freund ([email protected]) wrote:
> On 2023-10-18 15:53:30 -0400, Stephen Frost wrote:
> > > Here how pg_backend_memory_contexts would look like with this patch:
> > >
> > > postgres=# SELECT name, id, parent, parent_id, path
> > > FROM pg_backend_memory_contexts
> > > ORDER BY total_bytes DESC LIMIT 10;
> > > name | id | parent | parent_id | path
> > > -------------------------+-----+------------------+-----------+--------------
> > > CacheMemoryContext | 27 | TopMemoryContext | 0 | {0}
> > > Timezones | 124 | TopMemoryContext | 0 | {0}
> > > TopMemoryContext | 0 | | |
> > > MessageContext | 8 | TopMemoryContext | 0 | {0}
> > > WAL record construction | 118 | TopMemoryContext | 0 | {0}
> > > ExecutorState | 18 | PortalContext | 17 | {0,16,17}
> > > TupleSort main | 19 | ExecutorState | 18 | {0,16,17,18}
> > > TransactionAbortContext | 14 | TopMemoryContext | 0 | {0}
> > > smgr relation table | 10 | TopMemoryContext | 0 | {0}
> > > GUC hash table | 123 | GUCMemoryContext | 122 | {0,122}
> > > (10 rows)
> > >
> > > An example query to calculate the total_bytes including its children for a
> > > context (say CacheMemoryContext) would look like this:
> > >
> > > WITH contexts AS (
> > > SELECT * FROM pg_backend_memory_contexts
> > > )
> > > SELECT sum(total_bytes)
> > > FROM contexts
> > > WHERE ARRAY[(SELECT id FROM contexts WHERE name = 'CacheMemoryContext')] <@
> > > path;
> >
> > I wonder if we should perhaps just include
> > "total_bytes_including_children" as another column? Certainly seems
> > like a very useful thing that folks would like to see.
>
> The "issue" is where to stop - should we also add that for some of the other
> columns? They are a bit less important, but not that much.
I'm not sure the others really make sense to aggregate in this way as
free space isn't able to be moved between contexts. That said, if
someone wants it then I'm not against that. I'm actively in support of
adding an aggregated total though as that, at least to me, seems to be
very useful to have.
Thanks,
Stephen
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 19+ messages in thread
* [PATCH v5 5/7] Row pattern recognition patch (docs).
@ 2023-09-02 06:32 Tatsuo Ishii <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: Tatsuo Ishii @ 2023-09-02 06:32 UTC (permalink / raw)
---
doc/src/sgml/advanced.sgml | 52 ++++++++++++++++++++++++++++++++++
doc/src/sgml/func.sgml | 54 ++++++++++++++++++++++++++++++++++++
doc/src/sgml/ref/select.sgml | 38 +++++++++++++++++++++++--
3 files changed, 142 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/advanced.sgml b/doc/src/sgml/advanced.sgml
index 755c9f1485..eda3612822 100644
--- a/doc/src/sgml/advanced.sgml
+++ b/doc/src/sgml/advanced.sgml
@@ -537,6 +537,58 @@ WHERE pos < 3;
<literal>rank</literal> less than 3.
</para>
+ <para>
+ Row pattern common syntax can be used with row pattern common syntax to
+ perform row pattern recognition in a query. Row pattern common syntax
+ includes two sub clauses. <literal>DEFINE</literal> defines definition
+ variables along with an expression. The expression must be a logical
+ expression, which means it must
+ return <literal>TRUE</literal>, <literal>FALSE</literal>
+ or <literal>NULL</literal>. Moreover if the expression comprises a column
+ reference, it must be the argument of <function>rpr</function>. An example
+ of <literal>DEFINE</literal> is as follows.
+
+<programlisting>
+DEFINE
+ LOWPRICE AS price <= 100,
+ UP AS price > PREV(price),
+ DOWN AS price < PREV(price)
+</programlisting>
+
+ Note that <function>PREV</function> returns price column in the previous
+ row if it's called in a context of row pattern recognition. So in the
+ second line means the definition variable "UP" is <literal>TRUE</literal>
+ when price column in the current row is greater than the price column in
+ the previous row. Likewise, "DOWN" is <literal>TRUE</literal> when when
+ price column in the current row is lower than the price column in the
+ previous row.
+ </para>
+ <para>
+ Once <literal>DEFINE</literal> exists, <literal>PATTERN</literal> can be
+ used. <literal>PATTERN</literal> defines a sequence of rows that satisfies
+ certain conditions. For example following <literal>PATTERN</literal>
+ defines that a row starts with the condition "LOWPRICE", then one or more
+ rows satisfy "UP" and finally one or more rows satisfy "DOWN". If a
+ sequence of rows found, rpr returns the column at the starting row.
+ Example of a <literal>SELECT</literal> using the <literal>DEFINE</literal>
+ and <literal>PATTERN</literal> clause is as follows.
+
+<programlisting>
+SELECT company, tdate, price, max(price) OVER w FROM stock
+ WINDOW w AS (
+ PARTITION BY company
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ AFTER MATCH SKIP PAST LAST ROW
+ INITIAL
+ PATTERN (LOWPRICE UP+ DOWN+)
+ DEFINE
+ LOWPRICE AS price <= 100,
+ UP AS price > PREV(price),
+ DOWN AS price < PREV(price)
+);
+</programlisting>
+ </para>
+
<para>
When a query involves multiple window functions, it is possible to write
out each one with a separate <literal>OVER</literal> clause, but this is
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 7a0d4b9134..b7bfc9271e 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -21772,6 +21772,7 @@ SELECT count(*) FROM sometable;
returns <literal>NULL</literal> if there is no such row.
</para></entry>
</row>
+
</tbody>
</tgroup>
</table>
@@ -21811,6 +21812,59 @@ SELECT count(*) FROM sometable;
Other frame specifications can be used to obtain other effects.
</para>
+ <para>
+ Row pattern recognition navigation functions are listed in
+ <xref linkend="functions-rpr-navigation-table"/>. These functions
+ can be used to describe DEFINE clause of Row pattern recognition.
+ </para>
+
+ <table id="functions-rpr-navigation-table">
+ <title>Row Pattern Navigation Functions</title>
+ <tgroup cols="1">
+ <thead>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ Function
+ </para>
+ <para>
+ Description
+ </para></entry>
+ </row>
+ </thead>
+
+ <tbody>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>prev</primary>
+ </indexterm>
+ <function>prev</function> ( <parameter>value</parameter> <type>anyelement</type> )
+ <returnvalue>anyelement</returnvalue>
+ </para>
+ <para>
+ Returns the column value at the previous row;
+ returns NULL if there is no previous row in the window frame.
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>next</primary>
+ </indexterm>
+ <function>next</function> ( <parameter>value</parameter> <type>anyelement</type> )
+ <returnvalue>anyelement</returnvalue>
+ </para>
+ <para>
+ Returns the column value at the next row;
+ returns NULL if there is no next row in the window frame.
+ </para></entry>
+ </row>
+
+ </tbody>
+ </tgroup>
+ </table>
+
<note>
<para>
The SQL standard defines a <literal>RESPECT NULLS</literal> or
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 0ee0cc7e64..8d3becd57a 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -966,8 +966,8 @@ WINDOW <replaceable class="parameter">window_name</replaceable> AS ( <replaceabl
The <replaceable class="parameter">frame_clause</replaceable> can be one of
<synopsis>
-{ RANGE | ROWS | GROUPS } <replaceable>frame_start</replaceable> [ <replaceable>frame_exclusion</replaceable> ]
-{ RANGE | ROWS | GROUPS } BETWEEN <replaceable>frame_start</replaceable> AND <replaceable>frame_end</replaceable> [ <replaceable>frame_exclusion</replaceable> ]
+{ RANGE | ROWS | GROUPS } <replaceable>frame_start</replaceable> [ <replaceable>frame_exclusion</replaceable> ] [row_pattern_common_syntax]
+{ RANGE | ROWS | GROUPS } BETWEEN <replaceable>frame_start</replaceable> AND <replaceable>frame_end</replaceable> [ <replaceable>frame_exclusion</replaceable> ] [row_pattern_common_syntax]
</synopsis>
where <replaceable>frame_start</replaceable>
@@ -1074,6 +1074,40 @@ EXCLUDE NO OTHERS
a given peer group will be in the frame or excluded from it.
</para>
+ <para>
+ The
+ optional <replaceable class="parameter">row_pattern_common_syntax</replaceable>
+ defines the <firstterm>row pattern recognition condition</firstterm> for
+ this
+ window. <replaceable class="parameter">row_pattern_common_syntax</replaceable>
+ includes following subclauses. <literal>AFTER MATCH SKIP PAST LAST
+ ROW</literal> or <literal>AFTER MATCH SKIP TO NEXT ROW</literal> controls
+ how to proceed to next row position after a match
+ found. With <literal>AFTER MATCH SKIP PAST LAST ROW</literal> (the
+ default) next row position is next to the last row of previous match. On
+ the other hand, with <literal>AFTER MATCH SKIP TO NEXT ROW</literal> next
+ row position is always next to the last row of previous
+ match. <literal>DEFINE</literal> defines definition variables along with a
+ boolean expression. <literal>PATTERN</literal> defines a sequence of rows
+ that satisfies certain conditions using variables defined
+ in <literal>DEFINE</literal> clause. If the variable is not defined in
+ the <literal>DEFINE</literal> clause, it is implicitly assumed
+ following is defined in the <literal>DEFINE</literal> clause.
+
+<synopsis>
+<literal>variable_name</literal> AS TRUE
+</synopsis>
+
+ Note that the maximu number of variables defined
+ in <literal>DEFINE</literal> clause is 26.
+
+<synopsis>
+[ AFTER MATCH SKIP PAST LAST ROW | AFTER MATCH SKIP TO NEXT ROW ]
+PATTERN <replaceable class="parameter">pattern_variable_name</replaceable>[+] [, ...]
+DEFINE <replaceable class="parameter">definition_varible_name</replaceable> AS <replaceable class="parameter">expression</replaceable> [, ...]
+</synopsis>
+ </para>
+
<para>
The purpose of a <literal>WINDOW</literal> clause is to specify the
behavior of <firstterm>window functions</firstterm> appearing in the query's
--
2.25.1
----Next_Part(Sat_Sep__2_15_52_35_2023_273)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v5-0006-Row-pattern-recognition-patch-tests.patch"
^ permalink raw reply [nested|flat] 19+ messages in thread
end of thread, other threads:[~2024-07-10 21:16 UTC | newest]
Thread overview: 19+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-06-16 14:03 Parent/child context relation in pg_get_backend_memory_contexts() Melih Mutlu <[email protected]>
2023-08-04 18:16 ` Melih Mutlu <[email protected]>
2023-10-12 16:23 ` Andres Freund <[email protected]>
2023-10-23 12:02 ` Melih Mutlu <[email protected]>
2023-12-04 04:43 ` torikoshia <[email protected]>
2024-01-03 11:40 ` Melih Mutlu <[email protected]>
2024-01-10 06:37 ` torikoshia <[email protected]>
2024-01-16 09:41 ` Melih Mutlu <[email protected]>
2024-01-19 08:41 ` torikoshia <[email protected]>
2024-02-14 07:23 ` Michael Paquier <[email protected]>
2024-04-03 13:20 ` Melih Mutlu <[email protected]>
2024-04-03 23:34 ` Michael Paquier <[email protected]>
2024-04-04 01:44 ` David Rowley <[email protected]>
2024-07-02 13:08 ` Melih Mutlu <[email protected]>
2024-07-10 21:16 ` Robert Haas <[email protected]>
2023-10-18 19:53 ` Stephen Frost <[email protected]>
2023-10-19 01:17 ` Andres Freund <[email protected]>
2023-10-19 22:01 ` Stephen Frost <[email protected]>
2023-09-02 06:32 [PATCH v5 5/7] Row pattern recognition patch (docs). 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