On Tue, Dec 14, 2021 at 6:23 PM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>
> 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
"subtransactioncache of backend overflowed". But, when these functions are called it shows these stats for the
non-backendprocesses 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
thenewly 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"?
Thanks, Ashutosh, I will work on your comments and post an updated
version by next week.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com