Re: Add sub-transaction overflow status in pg_stat_activity - Mailing list pgsql-hackers
From | Ashutosh Sharma |
---|---|
Subject | Re: Add sub-transaction overflow status in pg_stat_activity |
Date | |
Msg-id | CAE9k0PnP5zj6Bz6xDJw63G429zcX4rjDEB=61h=q1X2ezB6erg@mail.gmail.com Whole thread Raw |
In response to | Re: Add sub-transaction overflow status in pg_stat_activity (Dilip Kumar <dilipbalaut@gmail.com>) |
Responses |
Re: Add sub-transaction overflow status in pg_stat_activity
Re: Add sub-transaction overflow status in pg_stat_activity |
List | pgsql-hackers |
Hi,
I have looked into the v2 patch and here are my comments:
+ PG_RETURN_INT32(local_beentry->subxact_overflowed);
+}
Should this be PG_RETURN_BOOL instead of PG_RETURN_INT32??
--
+{ oid => '6107', descr => 'statistics: cached subtransaction count of backend',
+ proname => 'pg_stat_get_backend_subxact_count', provolatile => 's', proparallel => 'r',
+ prorettype => 'int4', proargtypes => 'int4',
+ prosrc => 'pg_stat_get_backend_subxact_count' },
+{ oid => '6108', descr => 'statistics: subtransaction cache of backend overflowed',
+ proname => 'pg_stat_get_backend_subxact_overflow', provolatile => 's', proparallel => 'r',
+ prorettype => 'bool', proargtypes => 'int4',
+ prosrc => 'pg_stat_get_backend_subxact_overflow' },
The description says that the two new functions show the statistics for "cached subtransaction count of backend" and "subtransaction cache of backend overflowed". But, when these functions are called it shows these stats for the non-backend processes like checkpointer, walwriter etc as well. Should that happen?
--
typedef struct LocalPgBackendStatus
* not.
*/
TransactionId backend_xmin;
+
+ /*
+ * Number of cached subtransactions in the current session.
+ */
+ int subxact_count;
+
+ /*
+ * The number of subtransactions in the current session exceeded the cached
+ * subtransaction limit.
+ */
+ bool subxact_overflowed;
All the variables inside this LocalPgBackendStatus structure are prefixed with "backend" word. Can we do the same for the newly added variables as well?
--
+ * Get the xid and xmin, nsubxid and overflow status of the backend. The
Should this be put as - "xid, xmin, nsubxid and overflow" instead of "xid and xmin, nsubxid and overflow"?
+}
Should this be PG_RETURN_BOOL instead of PG_RETURN_INT32??
--
+{ oid => '6107', descr => 'statistics: cached subtransaction count of backend',
+ proname => 'pg_stat_get_backend_subxact_count', provolatile => 's', proparallel => 'r',
+ prorettype => 'int4', proargtypes => 'int4',
+ prosrc => 'pg_stat_get_backend_subxact_count' },
+{ oid => '6108', descr => 'statistics: subtransaction cache of backend overflowed',
+ proname => 'pg_stat_get_backend_subxact_overflow', provolatile => 's', proparallel => 'r',
+ prorettype => 'bool', proargtypes => 'int4',
+ prosrc => 'pg_stat_get_backend_subxact_overflow' },
The description says that the two new functions show the statistics for "cached subtransaction count of backend" and "subtransaction cache of backend overflowed". But, when these functions are called it shows these stats for the non-backend processes like checkpointer, walwriter etc as well. Should that happen?
--
typedef struct LocalPgBackendStatus
* not.
*/
TransactionId backend_xmin;
+
+ /*
+ * Number of cached subtransactions in the current session.
+ */
+ int subxact_count;
+
+ /*
+ * The number of subtransactions in the current session exceeded the cached
+ * subtransaction limit.
+ */
+ bool subxact_overflowed;
All the variables inside this LocalPgBackendStatus structure are prefixed with "backend" word. Can we do the same for the newly added variables as well?
--
+ * Get the xid and xmin, nsubxid and overflow status of the backend. The
Should this be put as - "xid, xmin, nsubxid and overflow" instead of "xid and xmin, nsubxid and overflow"?
--
With Regards,
Ashutosh Sharma.
On Mon, Dec 13, 2021 at 7:58 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Tue, Dec 7, 2021 at 11:11 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> You added this to pg_stat_activity, which already has a lot of fields.
> We talked a few months ago about not adding more fields that weren't commonly
> used.
> https://www.postgresql.org/message-id/flat/20210426191811.sp3o77doinphyjhu%40alap3.anarazel.de#d96d0a116f0344301eead2676ea65b2e
>
> Since I think this field is usually not interesting to most users of
> pg_stat_activity, maybe this should instead be implemented as a function like
> pg_backend_get_subxact_status(pid).
>
> People who want to could use it like:
> SELECT * FROM pg_stat_activity psa, pg_backend_get_subxact_status(pid) sub;
I have provided two function, one for subtransaction counts and other
whether subtransaction cache is overflowed or not, we can use like
this, if we think this is better way to do it then we can also add
another function for the lastOverflowedXid
postgres[43994]=# select id, pg_stat_get_backend_pid(id) as pid,
pg_stat_get_backend_subxact_count(id) as nsubxact,
pg_stat_get_backend_subxact_overflow(id) as overflowed from
pg_stat_get_backend_idset() as id;
id | pid | nsubxact | overflowed
----+-------+----------+------------
1 | 43806 | 0 | f
2 | 43983 | 64 | t
3 | 43994 | 0 | f
4 | 44323 | 22 | f
5 | 43802 | 0 | f
6 | 43801 | 0 | f
7 | 43804 | 0 | f
(7 rows)
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
pgsql-hackers by date: