Re: Add connection active, idle time to pg_stat_activity - Mailing list pgsql-hackers

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