Re: Add session statistics to pg_stat_database

On Fri, 2020-12-25 at 20:28 +0900, Masahiro Ikeda wrote:
> As a user, I want this feature to know whether
> clients' session activities are as expected.
> 
> I have some comments about the patch.

Thanks you for the thorough review!

> 1. pg_proc.dat
> 
> The unit of "session time" and so on says "in seconds".
> But, is "in milliseconds" right?

You are right.  Fixed.

> 2. monitoring.sgml
> 
> IIUC, "active_time" includes the time executes a fast-path function and
> "idle in transaction" includes "idle in transaction(aborted)" time.
>
> Why don't you reference pg_stat_activity's "state" column and
> "active_time" is the total time when the state is "active" and "fast 
> path"?
> "idle in transaction" is as same too.

Good idea; I have expanded the documentation like that.

> 3. pgstat.h
> 
> The comment of PgStat_MsgConn says "Sent by pgstat_connection".
> I thought "pgstat_connection" is a function, but it doesn't exist.
>
> Is "Sent by the backend" right?

The function was renamed and is now called "pgstat_send_connstats".

But you are right, I might as well match the surrounding code and
write "Sent by the backend".

> Although this is a trivial thing, the following row has too many tabs.
> 
> Other structs have only one space.
> 
> // }<tab><tab><tab>Pgstat_MsgConn;

Yes, I messed that up during the pgindent run.  Fixed.

Patch version 11 is attached.

Yours,
Laurenz Albe