Re: Add connection active, idle time to pg_stat_activity

From: Andrei Zubkov <zubkov(at)moonset(dot)ru>
To: Sergey Dudoladov <sergey(dot)dudoladov(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Aleksander Alekseev <aleksander(at)timescale(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>
Subject: Re: Add connection active, idle time to pg_stat_activity
Date: 2024-02-12 12:30:58
Message-ID: dfd8e2b429a207b0d823387f7245c1e6d8da092a.camel@moonset.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Sergei,

> I still would like to maintaint the focus on committing the "idle in
transactions" part of pg_stat_session first.

Agreed.

I've done a review of version 0004. This version is applied successful
over ce571434ae7, installcheck passed. The behavior of pg_stat_session
view and corresponding function looks correct. I've didn't found any
issues in the code.

Notes about the current state of a patch:

Naming
the view and function names 'pg_stat_session' seems correct for this
particular scope of a patch. However possible future resource
consumption statistics are valid for all backends (vacuum for example).
Right now it is not clear for me if we can get resource statistics from
those backends while those are listed in the pg_stat_activity view but
renaming to something like 'pg_stat_backend' seems reasonable to me.

Docs
1. session states referenced in monitoring.sgml is not uniform with
those of the pg_stat_activity view.
monitoring.sgml:4635
monitoring.sgml:4644
+ Time in milliseconds this backend spent in the
<literal>running</literal> or <literal>fastpath</literal> state.
I think those states should be referenced uniformly with
pg_stat_activity.

2. Description of the 'pg_stat_get_session()' function might be as
follows:

Returns a row, showing statistics about the client backend with the
specified process ID, or one row per client backend if
<literal>NULL</literal> is specified. The fields returned are the
same as those of <structname>pg_stat_session</structname> view.

The main thought here is to get rid of 'each active backend' because
'active backend' looks like backend in the 'active' state.

Tests
Call to a non-existing function is depend on non-existence of a
function, which can't be guaranteed absolutely. How about to do some
kind of obvious error here? Couple examples follows:

SELECT 0/0;

- or -

DO $$
BEGIN
RAISE 'test error';
END;
$$ LANGUAGE plpgsql;

My personal choice would be the last one.

--
regards, Andrei Zubkov
Postgres Professional

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2024-02-12 12:49:46 Re: clarify equalTupleDescs()
Previous Message Pavlo Golub 2024-02-12 12:27:54 Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role