Re: Add connection active, idle time to pg_stat_activity

Hi,

On 6/13/22 4:51 PM, Sergey Dudoladov wrote:
> Hello,
>
> I've updated the patch in preparation for the upcoming commitfest.

I really like the idea of adding additional information like the ones in
this patch, so +1 for the patch.

As far the patch:

@@ -864,7 +864,9 @@ CREATE VIEW pg_stat_activity AS
             s.backend_xmin,
             S.query_id,
             S.query,
-            S.backend_type
+            S.backend_type,
+            S.active_time,
+            S.idle_in_transaction_time

what about using total_active_time and total_idle_in_transaction_time?

I think that would avoid any confusion and "total_" is also already used
in other pg_stat_* views when appropriate.

@@ -468,6 +468,13 @@ pgstat_beshutdown_hook(int code, Datum arg)

        beentry->st_procpid = 0;        /* mark invalid */

+       /*
+        * Reset per-backend counters so that accumulated values for the
current
+        * backend are not used for future backends.
+        */
+       beentry->st_total_active_time = 0;
+       beentry->st_total_transaction_idle_time = 0;

shouldn't that be in pgstat_bestart() instead? (and just let
pgstat_beshutdown_hook() set st_procpid to 0)

        /* so that functions can check if backend_status.c is up via
MyBEEntry */
@@ -524,6 +531,8 @@ pgstat_report_activity(BackendState state, const
char *cmd_str)
        TimestampTz start_timestamp;
        TimestampTz current_timestamp;
        int                     len = 0;
+       int64           active_time_diff = 0;
+       int64           transaction_idle_time_diff = 0;

I think here we can use only a single variable say "state_time_diff" for
example, as later only one of those two is incremented anyway.

+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -539,7 +539,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS)
 Datum
 pg_stat_get_activity(PG_FUNCTION_ARGS)
 {
-#define PG_STAT_GET_ACTIVITY_COLS      30
+#define PG_STAT_GET_ACTIVITY_COLS      32
        int                     num_backends =
pgstat_fetch_stat_numbackends();
        int                     curr_backend;
        int                     pid = PG_ARGISNULL(0) ? -1 :
PG_GETARG_INT32(0);
@@ -621,6 +621,7 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
                {
                        SockAddr        zero_clientaddr;
                        char       *clipped_activity;
+                       int64           time_to_report;

what about total_time_to_report instead?

Also, maybe not for this patch but I think that would be also useful to
get the total time waited (so that we would get more inside of what the
"active" time was made of).

Regards,

--

Bertrand Drouvot
Amazon Web Services:https://aws.amazon.com