Re: Add session statistics to pg_stat_database

On Tue, Oct 13, 2020 at 01:44:41PM +0200, Laurenz Albe wrote:
> Attached is v3 with improvements.

+ <para>
+ Time spent in database sessions in this database, in milliseconds.
+ </para></entry>

Should say "Total time spent *by* DB sessions..." ?

I think these counters are only accurate as of the last state change, right?
So a session which has been idle for 1hr, that 1hr is not included. I think
the documentation should explain that, or (ideally) the implementation would be
more precise. Maybe the timestamps should only be updated after a session
terminates (and the docs should say so).

+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>connections</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Number of connections established to this database.

*Total* number of connections established, otherwise it sounds like it might
mean "the number of sessions [currently] established".

+ Number of database sessions to this database that did not end
+ with a regular client disconnection.

Does that mean "sessions which ended irregularly" ? Or does it also include
"sessions which have not ended" ?

+ msg.m_aborted = (!disconnect || pgStatSessionDisconnected) ? 0 : 1;

I think this can be just:
msg.m_aborted = (bool) (disconnect && !pgStatSessionDisconnected);

+ if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) == NULL)
+ result = 0;
+ else
+ result = ((double) dbentry->n_session_time) / 1000.0;

I think these can say:
|double result = 0;
|if ((dbentry=..) != NULL)
| result = (double) ..;

That not only uses fewer LOC, but also the assignment to zero is (known to be)
done at compile time (BSS) rather than runtime.