Re: Add session statistics to pg_stat_database

From: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
To: Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, Ahsan Hadi <ahsan(dot)hadi(at)gmail(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Soumyadeep Chakraborty <soumyadeep2007(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add session statistics to pg_stat_database
Date: 2021-01-08 09:34:24
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, 2021-01-08 at 12:00 +0900, Masahiro Ikeda wrote:
> 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.
> BTW, is there any reason to merge the above statistics?
> IIUC, to separate statistics' cons is that two columns increase, and
> there is no performance penalty. So, I wonder that there is a way to
> separate them
> corresponding to the state column of pg_stat_activity.

Sure, that could be done.

I decided to do it like this because I thought that few people would
be interested in "time spend doing fast-path function calls"; my guess
was that the more interesting value is "time where the database was
busy calculating results".

I tried to keep the balance between providing reasonable detail
while not creating more additional columns to "pg_stat_database"
than necessary.

This is of course a matter of taste, and it is good to hear different
opinions. If more people share your opinion, I'll change the code.

> There are some following codes in pgstatfuncs.c.
> int64 result = 0.0;
> But, I think the following is better.
> int64 result = 0;

You are right. That was a silly copy-and-paste error. Fixed.

> Although now pg_stat_get_db_session_time is initialize "result" to zero
> when it is declared,
> another pg_stat_XXX function didn't initialize. Is it better to change
> it?

I looked at other similar functions, and the ones I saw returned
NULL if there were no data. In that case, it makes sense to write

char *result;

if ((result = get_stats_data()) == NULL)


But I want to return 0 for the session time if there are no data yet,
so I think initializing the result to 0 in the declaration makes sense.

There are some functions that do it like this:

int32 result;

result = 0;
for (...)
if (...)


Again, it is a matter of taste, and I didn't detect a clear pattern
in the existing code that I feel I should follow in this question.

Version 12 of the patch is attached.

Laurenz Albe

Attachment Content-Type Size
0001-Add-session-statistics-to-pg_stat_database.v12.patch text/x-patch 22.8 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2021-01-08 09:35:28 Re: proposal - psql - use pager for \watch command
Previous Message Peter Smith 2021-01-08 09:25:15 Re: Single transaction in the tablesync worker?