Date: Fri, 28 Jan 2022 14:36:31 +0900 (JST)
Message-Id: <20220128.143631.654438319295147801.horikyota.ntt@gmail.com>
To: rjuju123@gmail.com
Cc: sergey.dudoladov@gmail.com, pgsql-hackers@lists.postgresql.org,
 kuntalghosh.2007@gmail.com, rafia.pghackers@gmail.com
Subject: Re: Add connection active, idle time to pg_stat_activity
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <20220127123656.etovnhimr5sgp37f@jrouhaud>
	<CAGz5QCLxnOpcPdjz-P0N_e3oT7SP0rz+Z9VmXCzKNywMpmHpqQ@mail.gmail.com>
References: <20220125142245.w5ezlmpxeg47xz66@jrouhaud>
	<CAA8Fd-qyHi5PbaUhKci+NA0N_j5J9MY2xOQ=ZznjzNhvz91QRA@mail.gmail.com>
	<20220127123656.etovnhimr5sgp37f@jrouhaud>
User-Agent: Mew version 6.8 on Emacs 26.1
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20220128.143631.654438319295147801.horikyota.ntt%40gmail.com>
Precedence: bulk

Hi.

At Thu, 27 Jan 2022 20:36:56 +0800, Julien Rouhaud <rjuju123@gmail.com> wrote in
> On Thu, Jan 27, 2022 at 11:43:26AM +0100, Sergey Dudoladov wrote:
> > 
> > Per agreement with Rafia I have reworked the patch in the past days.
> > The new version 6 is now ready for review.
> 
> Great, thanks a lot Sergey!
> 
> The cfbot is happy with this new version:
> https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/36/3405

I think we can easily add the duration of the current state to the two
in pg_stat_get_activity and it would offer better information.


 		if (beentry->st_state == STATE_RUNNING ||
 			beentry->st_state == STATE_FASTPATH)
-			pgstat_count_conn_active_time((PgStat_Counter) secs * 1000000 + usecs);
+		{
+			pgstat_count_conn_active_time((PgStat_Counter) usecs_diff);
+			beentry->st_total_active_time += usecs_diff;
+		}

The two lines operates exactly the same way on variables with slightly
different behavior. pgStatActiveTime is reported at transaction end
and reset at every tabstat reporting. st_total_active_time is reported
immediately and reset at session end. Since we do the latter, the
first can be omitted by remembering the last values for the local
variables at every reporting.  This needs additional two exporting
function in pgstatfuncs like pgstat_get_my_queryid so others might
think differently.

The write operation to beentry needs to be enclosed by
PGSTAT_BEGIN/END_WRITE_ACTIVITY().  In that perspective, it would be
better to move that writes to the PGSTAT_WRITE_ACTIVITY section just
below.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center