Thread
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Add system view pg_stat_kind_info
- 3b066de6c0a1 master landed
-
Introduce pluggable APIs for Cumulative Statistics
- 7949d9594582 18.0 cited
-
Add pg_stat_kind_info system view
Tristan Partin <tristan@partin.io> — 2026-04-30T17:47:59Z
Hey hackers, In 7949d959458[0], we introduced pluggable cumulative statistics. Extensions could now register their own custom statistics. However, there was no way for an extension author to introspect their custom statistics through SQL, and I don't believe there is currently a way to introspect builtin statistics either. Additionally, there was no way to know how much shared memory each statistics kind was using. We could only know the total shared memory used by the statistics subsystem. dbltap@postgres=# SELECT * FROM pg_shmem_allocations WHERE name = 'Shared Memory Stats'; name | off | size | allocated_size ---------------------+-----------+--------+---------------- Shared Memory Stats | 153456640 | 321976 | 321976 In this patch, I have added a new system view: pg_stat_kind_info built on a new function pg_stat_get_kind_info(). The view has the following columns: - id: id of the statistics kind - name: name of the statistics kind - count: number of entries for the statistics kind - builtin: whether the statistics kind of builtin or not - shared_size: shared memory size of each entry dbltap@postgres=# SELECT * FROM pg_stat_kind_info; id | name | count | builtin | shared_size ----+-------------------------+--------+---------+------------- 1 | database | (null) | t | 288 2 | relation | (null) | t | 248 3 | function | (null) | t | 56 4 | replslot | (null) | t | 120 5 | subscription | (null) | t | 120 6 | backend | (null) | t | 2952 7 | archiver | 1 | t | 0 8 | bgwriter | 1 | t | 0 9 | checkpointer | 1 | t | 0 10 | io | 1 | t | 0 11 | lock | 1 | t | 0 12 | slru | 1 | t | 0 13 | wal | 1 | t | 0 25 | test_custom_var_stats | 0 | f | 40 26 | test_custom_fixed_stats | 1 | f | 56 (15 rows) I am not sure that shared_size is a good column name, and I needed to include pgstat_internal.h in pgstatfuncs.c to get everything working. I'm also curious to hear if anyone thinks there is other valuable information to expose. [0]: https://github.com/postgres/postgres/commit/7949d9594582ab49dee221e1db1aa5401ace49d4 -- Tristan Partin PostgreSQL Contributors Team AWS (https://aws.amazon.com)
-
Re: Add pg_stat_kind_info system view
Tristan Partin <tristan@partin.io> — 2026-04-30T19:38:57Z
Of course I forgot to attach the patch :D. -- Tristan Partin PostgreSQL Contributors Team AWS (https://aws.amazon.com)
-
Re: Add pg_stat_kind_info system view
Michael Paquier <michael@paquier.xyz> — 2026-05-14T05:29:57Z
On Thu, Apr 30, 2026 at 07:38:57PM +0000, Tristan Partin wrote: > Of course I forgot to attach the patch :D. Thanks for the patch. That sounds like a good idea particularly for track_entry_count, because we have no real way to provide this information, which would be more valuable if an in-core stats kind has the idea to switch this flag to true in the future, and I don't really want all extensions to re-implement the same thing to access this information. + if (info->track_entry_count) + { + values[2] = Int64GetDatum(pgstat_get_entry_count(kind)); + } + else + { + nulls[2] = true; + } Returning NULL if the flag is not set sounds sensible. For fixed-numbered, fine by me for 1, so as it is possible to aggregate the full size counting with the fixed shmem size of each stats kind. +SELECT name, builtin FROM pg_stat_kind_info + ORDER BY name COLLATE "C"; This is not going to be stable if running installcheck on an instance where a custom kind is loaded, so let's restrict the query to check for built-in kinds. I'd suggest to expand the data published to more fields and not only what's presented here, so as it becomes possible to do more SQL sanity checks with the stats kind info (same attribute name as the fields): - fixed_amount, where shared_size > 0 does not make sense. - snapshot_ctl_off and shared_ctl_off, that only makes sense under fixed_amount. These should not be set for !fixed_amount. - pending_size, that should not be set for fixed_amount. - existence of flush_pending_cb, delete_pending_cb, reset_timestamp_cb, to_serialized_name, from_serialized_name, to_serialized_data, from_serialized_data (should be booleans), fine for !fixed_amount, never for fixed_amount. - existence of init_shmem_cb, reset_all_cb, snapshot_cb (should be booleans), fine for fixed_amount, never for !fixed_amount. I may be missing one or two things. pgstat_internal.h documents all these requirements, the idea is to translate these requirements at SQL level. We should definitely apply these checks for both custom and built-in stats kinds, which should save time for developers of pgstats in core and outside of core by detecting inconsistent patterns beforehand. -- Michael -
Re: Add pg_stat_kind_info system view
Tristan Partin <tristan@partin.io> — 2026-05-28T17:11:42Z
On Thu May 14, 2026 at 5:30 AM UTC, Michael Paquier wrote: > On Thu, Apr 30, 2026 at 07:38:57PM +0000, Tristan Partin wrote: >> Of course I forgot to attach the patch :D. > > Thanks for the patch. That sounds like a good idea particularly for > track_entry_count, because we have no real way to provide this > information, which would be more valuable if an in-core stats kind has > the idea to switch this flag to true in the future, and I don't really > want all extensions to re-implement the same thing to access this > information. > > + if (info->track_entry_count) > + { > + values[2] = Int64GetDatum(pgstat_get_entry_count(kind)); > + } > + else > + { > + nulls[2] = true; > + } > > Returning NULL if the flag is not set sounds sensible. For > fixed-numbered, fine by me for 1, so as it is possible to aggregate > the full size counting with the fixed shmem size of each stats kind. Awesome. > +SELECT name, builtin FROM pg_stat_kind_info > + ORDER BY name COLLATE "C"; > > This is not going to be stable if running installcheck on an instance > where a custom kind is loaded, so let's restrict the query to check > for built-in kinds. I'll fix this in v2, which I will send after resolving more discussion. I had not considered this as a potential problem. I'll remove the builtin column from the SELECT and filter on builtin as suggested. > I'd suggest to expand the data published to more fields and not only > what's presented here, so as it becomes possible to do more SQL sanity > checks with the stats kind info (same attribute name as the fields): > - fixed_amount, where shared_size > 0 does not make sense. > - snapshot_ctl_off and shared_ctl_off, that only makes sense under > fixed_amount. These should not be set for !fixed_amount. > - pending_size, that should not be set for fixed_amount. > - existence of flush_pending_cb, delete_pending_cb, > reset_timestamp_cb, to_serialized_name, from_serialized_name, > to_serialized_data, from_serialized_data (should be booleans), fine > for !fixed_amount, never for fixed_amount. > - existence of init_shmem_cb, reset_all_cb, snapshot_cb (should be > booleans), fine for fixed_amount, never for !fixed_amount. Can you share how someone might use this additional information? I can see some inherent value for additionally exposing: - fixed_amount - accessed_across_databases - write_to_file - snapshot_ctl_off - shared_ctl_off - shared_data_off - shared_data_len - pending_size > I may be missing one or two things. pgstat_internal.h documents all > these requirements, the idea is to translate these requirements at SQL > level. We should definitely apply these checks for both custom and > built-in stats kinds, which should save time for developers of pgstats > in core and outside of core by detecting inconsistent patterns > beforehand. Not sure I understand why we would want to expose the existence of the callbacks to make assertions at the SQL level. I see that in pgstat_register_kind(), we have the following code: /* * Check some data for fixed-numbered stats. */ if (kind_info->fixed_amount) { if (kind_info->shared_size == 0) ereport(ERROR, (errmsg("custom cumulative statistics property is invalid"), errhint("Custom cumulative statistics require a shared memory size for fixed-numbered objects."))); if (kind_info->track_entry_count) ereport(ERROR, (errmsg("custom cumulative statistics property is invalid"), errhint("Custom cumulative statistics cannot use entry count tracking for fixed-numbered objects."))); } We could extend these invariant checks to make sure that the callbacks are only set when fixed_amount is true for instance. I am very much open to having my mind changed. -- Tristan Partin PostgreSQL Contributors Team AWS (https://aws.amazon.com) -
Re: Add pg_stat_kind_info system view
Michael Paquier <michael@paquier.xyz> — 2026-06-01T05:40:54Z
On Thu, May 28, 2026 at 05:11:42PM +0000, Tristan Partin wrote: > Can you share how someone might use this additional information? > > Not sure I understand why we would want to expose the existence of the > callbacks to make assertions at the SQL level. I see that in > pgstat_register_kind(), we have the following code: > > We could extend these invariant checks to make sure that the callbacks > are only set when fixed_amount is true for instance. I am very much open > to having my mind changed. There is currently no internal mechanism to make sure that the built-in stats kinds have a consistent setup in terms of flags and callbacks set, so for developers we could immediately complain when generating patches that add new stats kinds. For custom stats kinds, loaded libraries could have more cross-checks. Perhaps it is not worth bothering at the end, and I'd be fine to keep the data published as minimal as you see fit. Still, fixed_amount, write_to_file and accessed_across_databases feel like useful additions. If we keep shared_size, it may make sense to set it to NULL if we don't know about it? That's the case of the built-in fixed-sized stats kinds. We set the value for custom fixed-sized stats kinds. -- Michael
-
Re: Add pg_stat_kind_info system view
Tristan Partin <tristan@partin.io> — 2026-06-04T16:59:00Z
On Mon Jun 1, 2026 at 5:41 AM UTC, Michael Paquier wrote: > On Thu, May 28, 2026 at 05:11:42PM +0000, Tristan Partin wrote: >> Can you share how someone might use this additional information? >> >> Not sure I understand why we would want to expose the existence of the >> callbacks to make assertions at the SQL level. I see that in >> pgstat_register_kind(), we have the following code: >> >> We could extend these invariant checks to make sure that the callbacks >> are only set when fixed_amount is true for instance. I am very much open >> to having my mind changed. > > There is currently no internal mechanism to make sure that the > built-in stats kinds have a consistent setup in terms of flags and > callbacks set, so for developers we could immediately complain when > generating patches that add new stats kinds. For custom stats kinds, > loaded libraries could have more cross-checks. I think there is still some confusion on my end about this line of discussion. > Perhaps it is not worth bothering at the end, and I'd be fine to keep > the data published as minimal as you see fit. Still, fixed_amount, > write_to_file and accessed_across_databases feel like useful > additions. The additional columns are now added in v2. Note that I named write_to_file as written_to_file for the column name. I wonder if persisted would be a better name for the column and if persisted would be a better name for PgStat_KindInfo::write_to_file. > If we keep shared_size, it may make sense to set it to NULL if we > don't know about it? That's the case of the built-in fixed-sized > stats kinds. We set the value for custom fixed-sized stats kinds. I believe that I also captured this correctly in v2. -- Tristan Partin PostgreSQL Contributors Team AWS (https://aws.amazon.com)
-
Re: Add pg_stat_kind_info system view
Michael Paquier <michael@paquier.xyz> — 2026-06-08T00:16:13Z
On Thu, Jun 04, 2026 at 04:59:00PM +0000, Tristan Partin wrote: > I believe that I also captured this correctly in v2. No v2 of the patch has been attached. :D -- Michael
-
Re: Add pg_stat_kind_info system view
solai v <solai.cdac@gmail.com> — 2026-06-11T06:35:01Z
Hi all, On Wed, Jun 10, 2026 at 3:34 PM Tristan Partin <tristan@partin.io> wrote: > > Of course I forgot to attach the patch :D. > > -- I reviewed the v1 patch and had verified that both the view and the function are created successfully and return identical results. The builtin statistics kinds are exposed correctly, and the IDs and names are unique without any duplicates. The count column behaves as expected, returning NULL for statistics kinds that do not support entry count tracking and 1 for fixed statistics kinds. While reviewing the patch, I found it may be useful to expose a boolean column such as track_entry_count in pg_stat_kind_info that would explicitly indicate whether entry count tracking is enabled for a statistics kind and also would help users understand why the count column is NULL for certain statistics kinds. Exposing this internal property would make the view more self-explanatory and improve its usefulness for debugging. Apart from these suggestions, I did not find any issues with the current implementation. Looking forward to more feedback. Regards, Solai
-
Re: Add pg_stat_kind_info system view
Tristan Partin <tristan@partin.io> — 2026-06-18T16:54:36Z
On Mon Jun 8, 2026 at 12:16 AM UTC, Michael Paquier wrote: > On Thu, Jun 04, 2026 at 04:59:00PM +0000, Tristan Partin wrote: >> I believe that I also captured this correctly in v2. > > No v2 of the patch has been attached. :D I am not a very smart person! :facepalm: -- Tristan Partin PostgreSQL Contributors Team AWS (https://aws.amazon.com)
-
Re: Add pg_stat_kind_info system view
Michael Paquier <michael@paquier.xyz> — 2026-06-24T04:12:09Z
On Thu, Jun 18, 2026 at 04:54:36PM +0000, Tristan Partin wrote: > On Mon Jun 8, 2026 at 12:16 AM UTC, Michael Paquier wrote: >> On Thu, Jun 04, 2026 at 04:59:00PM +0000, Tristan Partin wrote: >>> I believe that I also captured this correctly in v2. >> >> No v2 of the patch has been attached. :D > > I am not a very smart person! :facepalm: Thanks for the patch. A small comment that I have is related to the use of the unaligned output of in stats.sql, where I doubt that we'll have a stats kind named with more characters than the existing ones. And there are a bunch of bools, which would be easier to read if aligned. "written_to_file" as attribute name of the SQL function is inconsistent. "write_to_file" would be. The rest looks pretty much OK, seen from here. -- Michael
-
Re: Add pg_stat_kind_info system view
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-06-24T05:31:12Z
Hi, On Wed, Jun 24, 2026 at 01:12:09PM +0900, Michael Paquier wrote: > On Thu, Jun 18, 2026 at 04:54:36PM +0000, Tristan Partin wrote: > > On Mon Jun 8, 2026 at 12:16 AM UTC, Michael Paquier wrote: > >> On Thu, Jun 04, 2026 at 04:59:00PM +0000, Tristan Partin wrote: > >>> I believe that I also captured this correctly in v2. > >> > >> No v2 of the patch has been attached. :D Thanks for the patch, that looks like an useful addition to me. A few comments: === 1 +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -30,6 +30,8 @@ #include "storage/procarray.h" #include "utils/acl.h" #include "utils/builtins.h" +#include "utils/pgstat_internal.h" But header in pgstat_internal.h mentions: " * Definitions for the PostgreSQL cumulative statistics system that should * only be needed by files implementing statistics support (rather than ones * reporting / querying stats). " So it looks like that we are breaking this convention here. Maybe add helper function(s)? === 2 + /* For fixed-amount kinds, count is always 1. The entry is stored in + * PgStat_ShmemControl. If it is not a fixed-amount, then report the + * count of entries if tracked, or NULL if not tracked. + */ comment style is not right. === 3 + /* For fixed-amount kinds, count is always 1. Why not report NULL for fixed-amount kinds too? The track_entry_count field is documented as being for variable-numbered stats only, so the count would only be reported for custom variable-numbered kinds that set it to true. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
-
Re: Add pg_stat_kind_info system view
Michael Paquier <michael@paquier.xyz> — 2026-06-30T06:42:40Z
On Wed, Jun 24, 2026 at 05:31:12AM +0000, Bertrand Drouvot wrote: > But header in pgstat_internal.h mentions: > > " > * Definitions for the PostgreSQL cumulative statistics system that should > * only be needed by files implementing statistics support (rather than ones > * reporting / querying stats). > > " > > So it looks like that we are breaking this convention here. Maybe add helper > function(s)? Yeah. Pulling pgstat_internal.h in pgstatfuncs.c is not great. The code acts as a barrier as the internal part and the fields data retrieved by the functions. A couple of alternatives I can think of: - Put this new function in a new file under activity, named blankly pgstat_kind.c. - Move the necessary stuff out of pgstat_internal.h into a new header, or just pgstat_kind.h with a !FRONTEND block. - Use a new wrapper function that copies the data we need into an intermediate structure. This is just a new pgstat_get_kind_info(). Among these two, putting pg_stat_get_kind_info() into a new file feels much better than splitting the contents of pgstat_internal.h, which is kind of the central in-core shmem-side facility. Using a wrapper as of option 3 looks weird knowing the existence of the kind_info() function. Bertrand? Tristan? -- Michael
-
Re: Add pg_stat_kind_info system view
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-06-30T07:59:04Z
Hi, On Tue, Jun 30, 2026 at 03:42:40PM +0900, Michael Paquier wrote: > On Wed, Jun 24, 2026 at 05:31:12AM +0000, Bertrand Drouvot wrote: > > But header in pgstat_internal.h mentions: > > > > " > > * Definitions for the PostgreSQL cumulative statistics system that should > > * only be needed by files implementing statistics support (rather than ones > > * reporting / querying stats). > > > > " > > > > So it looks like that we are breaking this convention here. Maybe add helper > > function(s)? > > Yeah. Pulling pgstat_internal.h in pgstatfuncs.c is not great. The > code acts as a barrier as the internal part and the fields data > retrieved by the functions. A couple of alternatives I can think of: > - Put this new function in a new file under activity, named blankly > pgstat_kind.c. > - Move the necessary stuff out of pgstat_internal.h into a new header, > or just pgstat_kind.h with a !FRONTEND block. > - Use a new wrapper function that copies the data we need into an > intermediate structure. This is just a new pgstat_get_kind_info(). > > Among these two, putting pg_stat_get_kind_info() into a new file feels > much better than splitting the contents of pgstat_internal.h, which is > kind of the central in-core shmem-side facility. I do agree. That said, creating a new file only for one function looks a bit weird (but I can not think of a better option though). Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
-
Re: Add pg_stat_kind_info system view
Michael Paquier <michael@paquier.xyz> — 2026-06-30T08:02:12Z
On Tue, Jun 30, 2026 at 07:59:04AM +0000, Bertrand Drouvot wrote: > I do agree. That said, creating a new file only for one function looks a bit > weird (but I can not think of a better option though). FWIW, I've done that in the past, when it was necessary. -- Michael
-
Re: Add pg_stat_kind_info system view
Sami Imseih <samimseih@gmail.com> — 2026-06-30T22:36:57Z
Hi, Thanks for working on this. I like this overall, but have some comments. 1/ + One row for each loaded statistics kind, showing metadata about the kind. The description says "showing metadata about the kind" but the count column returns live runtime data (via pgstat_get_entry_count()), not just metadata. The description should acknowledge both. 2/ + <structfield>shared_size</structfield> <type>bigint</type> + </para> + <para> + Size in bytes of a shared memory entry for this statistics kind. + <literal>NULL</literal> for built-in fixed-amount kinds, whose + single entry lives in a statically-allocated slot rather than a + sized shared memory entry. I think we should just use info->shared_data_len for both fixed and variable-length kinds. This shows the entry_size which is what an extension developer will care about. Even info->shared_size for variable-length kinds does not include the per-entry overhead, so info->shared_data_len will be useful to understand if the size of your entry changed after an upgrade, etc. Also, we should rename it to "entry_size" instead of "shared_size" 3/ + <structfield>count</structfield> <type>bigint</type> + </para> + <para> + Number of tracked entries for this kind. For fixed-amount kinds, this is + always 1. For variable-numbered kinds, this is the number of objects + currently tracked. <literal>NULL</literal> if the kind does not track + entry counts. + </para> + </entry> I think this should be 0 for all kinds by default. Only ever > 0 for variable-numbered kinds with entry tracking enabled. No NULLs. The fields should be called "entry_count" instead of "count". We should update the documentation to reflect that as well. That seems easier to query than having mixed NULLs and numbers. Sami Imseih Amazon Web Services (AWS)
-
Re: Add pg_stat_kind_info system view
Michael Paquier <michael@paquier.xyz> — 2026-07-01T01:35:33Z
On Tue, Jun 30, 2026 at 05:36:57PM -0500, Sami Imseih wrote: > + <structfield>count</structfield> <type>bigint</type> > + </para> > + <para> > + Number of tracked entries for this kind. For fixed-amount > kinds, this is > + always 1. For variable-numbered kinds, this is the number of objects > + currently tracked. <literal>NULL</literal> if the kind does not track > + entry counts. > + </para> > + </entry> > > I think this should be 0 for all kinds by default. Only ever > 0 for > variable-numbered > kinds with entry tracking enabled. No NULLs. The fields should be called > "entry_count" instead of "count". We should update the documentation to > reflect that as well. That seems easier to query than having mixed NULLs and > numbers. IMO, in this case, NULL should be a synonym of "I don't know", which is what entry_count set to false means. 0 means "I know, there is no data". I'd be OK with dropping the part about fixed-sized stats where we enforce 1, and use NULL instead, though. -- Michael
-
Re: Add pg_stat_kind_info system view
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-07-01T03:39:37Z
Hi, On Wed, Jul 01, 2026 at 10:35:33AM +0900, Michael Paquier wrote: > On Tue, Jun 30, 2026 at 05:36:57PM -0500, Sami Imseih wrote: > > + <structfield>count</structfield> <type>bigint</type> > > + </para> > > + <para> > > + Number of tracked entries for this kind. For fixed-amount > > kinds, this is > > + always 1. For variable-numbered kinds, this is the number of objects > > + currently tracked. <literal>NULL</literal> if the kind does not track > > + entry counts. > > + </para> > > + </entry> > > > > I think this should be 0 for all kinds by default. Only ever > 0 for > > variable-numbered > > kinds with entry tracking enabled. No NULLs. The fields should be called > > "entry_count" instead of "count". We should update the documentation to > > reflect that as well. That seems easier to query than having mixed NULLs and > > numbers. > > IMO, in this case, NULL should be a synonym of "I don't know", which > is what entry_count set to false means. 0 means "I know, there is no > data". I'd be OK with dropping the part about fixed-sized stats where > we enforce 1, and use NULL instead, though. +1, that's also my opinion [1]. [1]: https://postgr.es/m/ajtroNXfBS/ldtNZ%40bdtpg Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
-
Re: Add pg_stat_kind_info system view
Sami Imseih <samimseih@gmail.com> — 2026-07-01T04:17:29Z
> > > I think this should be 0 for all kinds by default. Only ever > 0 for > > > variable-numbered > > > kinds with entry tracking enabled. No NULLs. The fields should be called > > > "entry_count" instead of "count". We should update the documentation to > > > reflect that as well. That seems easier to query than having mixed NULLs and > > > numbers. > > > > IMO, in this case, NULL should be a synonym of "I don't know", which > > is what entry_count set to false means. 0 means "I know, there is no > > data". I'd be OK with dropping the part about fixed-sized stats where > > we enforce 1, and use NULL instead, though. > > +1, that's also my opinion [1]. > > [1]: https://postgr.es/m/ajtroNXfBS/ldtNZ%40bdtpg I'm ok with that if others feel this is better. -- Sami
-
Re: Add pg_stat_kind_info system view
Michael Paquier <michael@paquier.xyz> — 2026-07-01T07:07:06Z
On Tue, Jun 30, 2026 at 11:17:29PM -0500, Sami Imseih wrote: >>> IMO, in this case, NULL should be a synonym of "I don't know", which >>> is what entry_count set to false means. 0 means "I know, there is no >>> data". I'd be OK with dropping the part about fixed-sized stats where >>> we enforce 1, and use NULL instead, though. >> >> +1, that's also my opinion [1]. > > I'm ok with that if others feel this is better. Okay, thanks. Let's do so then. Let's also invent a new pgstat_kind.c in activity/. I have been chewing a bit on the comments from Sami, leading to the following result: - Switched shared_size to entry_size, for consistency with entry_count, but I don't agree about the use of shared_data_len. shared_size is more adapted to me because it has the entry overhead and the shmem entry header. That's more precise and one does not need to guess the header size. - Moved entry_count after entry_size in the list of attributes. - written_to_file -> write_to_file, same as pgstat_internal.h. - Moved the new function to a pgstat_kind.c. - Some extra changes and tweaks to the comments, the docs, some code. The attached should do all that, hopefully. Any thoughts? -- Michael
-
Re: Add pg_stat_kind_info system view
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-07-01T08:38:16Z
Hi, On Wed, Jul 01, 2026 at 04:07:06PM +0900, Michael Paquier wrote: > On Tue, Jun 30, 2026 at 11:17:29PM -0500, Sami Imseih wrote: > >>> IMO, in this case, NULL should be a synonym of "I don't know", which > >>> is what entry_count set to false means. 0 means "I know, there is no > >>> data". I'd be OK with dropping the part about fixed-sized stats where > >>> we enforce 1, and use NULL instead, though. > >> > >> +1, that's also my opinion [1]. > > > > I'm ok with that if others feel this is better. > > Okay, thanks. Let's do so then. Let's also invent a new > pgstat_kind.c in activity/. > > I have been chewing a bit on the comments from Sami, leading to the > following result: > - Switched shared_size to entry_size, for consistency with > entry_count, but I don't agree about the use of shared_data_len. > shared_size is more adapted to me because it has the entry overhead > and the shmem entry header. That's more precise and one does not need > to guess the header size. > - Moved entry_count after entry_size in the list of attributes. > - written_to_file -> write_to_file, same as pgstat_internal.h. > - Moved the new function to a pgstat_kind.c. > - Some extra changes and tweaks to the comments, the docs, some code. > > The attached should do all that, hopefully. Any thoughts? Yeah I also think it does all of that and LGTM. Nit: $ git show | grep -i loaded + One row for each loaded statistics kind, showing information about + loaded statistics kind, including both built-in and custom kinds. + shutdown and reloaded on startup, false if they are kept only in + * Get information about the statistics kinds loaded into the system. + descr => 'statistics: information about loaded statistics kinds', +-- List of loaded statistics kinds. +-- There should be at least one statistics kind loaded +-- List of loaded statistics kinds. +-- There should be at least one statistics kind loaded s/loaded/registered/? Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
-
Re: Add pg_stat_kind_info system view
Sami Imseih <samimseih@gmail.com> — 2026-07-01T17:21:39Z
> On Wed, Jul 01, 2026 at 04:07:06PM +0900, Michael Paquier wrote: > > On Tue, Jun 30, 2026 at 11:17:29PM -0500, Sami Imseih wrote: > > >>> IMO, in this case, NULL should be a synonym of "I don't know", which > > >>> is what entry_count set to false means. 0 means "I know, there is no > > >>> data". I'd be OK with dropping the part about fixed-sized stats where > > >>> we enforce 1, and use NULL instead, though. > > >> > > >> +1, that's also my opinion [1]. > > > > > > I'm ok with that if others feel this is better. > > > > Okay, thanks. Let's do so then. Let's also invent a new > > pgstat_kind.c in activity/. > > > > I have been chewing a bit on the comments from Sami, leading to the > > following result: > > - Switched shared_size to entry_size, for consistency with > > entry_count, but I don't agree about the use of shared_data_len. > > shared_size is more adapted to me because it has the entry overhead > > and the shmem entry header. That's more precise and one does not need > > to guess the header size. It still does not include PgStatShared_HashEntry which is 40-bytes per entry, so it could add up if someone is calculating total consumption by a kind as number_of_entries * size_of_entry. So that should also be accounted for, right? But also, because this will be used to calculate consumption, we should add a note in the documentation to differentiate between live storage usage vs the DSA footprint, which will not be shrunk when entries are deleted. Someone may be confused that their entries are much lower after deleting entries, but their memory footprint is still high because the OS does not reclaim the free'd space. What do you think? -- Sami
-
Re: Add pg_stat_kind_info system view
Michael Paquier <michael@paquier.xyz> — 2026-07-01T21:28:11Z
On Wed, Jul 01, 2026 at 12:21:39PM -0500, Sami Imseih wrote: > It still does not include PgStatShared_HashEntry which is 40-bytes per entry, > so it could add up if someone is calculating total consumption by a kind as > number_of_entries * size_of_entry. So that should also be accounted > for, right? Right. I forgot about this part. HashEntry stands on top of the PgStatShared_* structures. > But also, because this will be used to calculate consumption, we > should add a note in > the documentation to differentiate between live storage usage vs the > DSA footprint, > which will not be shrunk when entries are deleted. Someone may be confused > that their entries are much lower after deleting entries, but their memory > footprint is still high because the OS does not reclaim the free'd space. > What do you think? Hmm. I am not completely sure which way is best here. So, I think that I am just going to drop this field for now (entry_size as of latest patch), and keep the rest of the patch. It's still useful to me, and we could always add one or more memory-related field later as we feel in this release cycle. -- Michael
-
Re: Add pg_stat_kind_info system view
Sami Imseih <samimseih@gmail.com> — 2026-07-02T00:26:23Z
> > But also, because this will be used to calculate consumption, we > > should add a note in > > the documentation to differentiate between live storage usage vs the > > DSA footprint, > > which will not be shrunk when entries are deleted. Someone may be confused > > that their entries are much lower after deleting entries, but their memory > > footprint is still high because the OS does not reclaim the free'd space. > > What do you think? > > Hmm. I am not completely sure which way is best here. So, I think > that I am just going to drop this field for now (entry_size as of > latest patch), and keep the rest of the patch. It's still useful to > me, and we could always add one or more memory-related field later as > we feel in this release cycle. Right. I also think this needs more discussion, and it should not hold up everything else. -- Sami
-
Re: Add pg_stat_kind_info system view
Michael Paquier <michael@paquier.xyz> — 2026-07-02T00:37:43Z
On Wed, Jul 01, 2026 at 07:26:23PM -0500, Sami Imseih wrote: > Right. I also think this needs more discussion, and it should not hold > up everything else. Just done this way now as of 3b066de6c0a1, with more more adjustments, and without the sizing parts. -- Michael
-
Re: Add pg_stat_kind_info system view
Tristan Partin <tristan@partin.io> — 2026-07-06T22:52:02Z
On Thu Jul 2, 2026 at 12:37 AM UTC, Michael Paquier wrote: > On Wed, Jul 01, 2026 at 07:26:23PM -0500, Sami Imseih wrote: >> Right. I also think this needs more discussion, and it should not hold >> up everything else. > > Just done this way now as of 3b066de6c0a1, with more more adjustments, > and without the sizing parts. Thanks for committing Michael, and thanks for the reviews Bertrand and Sami. Here is a patch that add the aforementioned entry_size column. It is definitely needs further discussion. I am not entirely sure that I see the value of using PgStat_KindInfo::shared_size for this column, so I used PgStat_KindInfo::shared_data_len instead via pgstat_get_entry_len(). My reasoning for choosing so is: - An argument against shared_size is that I think trying to match up pg_stat_kind_info with pg_shmem_allocations will not work well because we will miss the additional hash table overhead - Additionally, a few builtin stats don't even report a shared_size (bgwriter, archiver, checkpointer, etc.) Looking forward to see what other ideas or reasonings you all might propose. -- Tristan Partin PostgreSQL Contributors Team AWS (https://aws.amazon.com)