Date: Fri, 28 Jan 2022 14:43:03 +0900 (JST)
Message-Id: <20220128.144303.648220302965090533.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: <20220128.143631.654438319295147801.horikyota.ntt@gmail.com>
References: <CAA8Fd-qyHi5PbaUhKci+NA0N_j5J9MY2xOQ=ZznjzNhvz91QRA@mail.gmail.com>
	<20220127123656.etovnhimr5sgp37f@jrouhaud>
	<20220128.143631.654438319295147801.horikyota.ntt@gmail.com>
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.144303.648220302965090533.horikyota.ntt%40gmail.com>
Precedence: bulk

At Fri, 28 Jan 2022 14:36:31 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in 
> 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

Of course it's typo(?) of "values of the shared variables".
Sorry for the mistake.

> 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