On Fri, 2021-09-03 at 17:04 -0700, Andres Freund wrote:
> Hi,
>
> On 2021-08-31 21:56:50 -0700, Andres Freund wrote:
> > On 2021-08-27 13:57:45 +0900, Michael Paquier wrote:
> > > On Wed, Aug 25, 2021 at 01:20:03AM -0700, Andres Freund wrote:
> > > > On 2021-08-25 12:51:58 +0900, Michael Paquier wrote:
> > > > As I said before, this ship has long sailed:
> > > >
> > > > typedef struct PgStat_MsgTabstat
> > > > {
> > > > PgStat_MsgHdr m_hdr;
> > > > Oid m_databaseid;
> > > > int m_nentries;
> > > > int m_xact_commit;
> > > > int m_xact_rollback;
> > > > PgStat_Counter m_block_read_time; /* times in
> > > > microseconds */
> > > > PgStat_Counter m_block_write_time;
> > > > PgStat_TableEntry m_entry[PGSTAT_NUM_TABENTRIES];
> > > > } PgStat_MsgTabstat;
> > >
> > > Well, I kind of misread what you meant upthread then.
> > > PgStat_MsgTabstat has a name a bit misleading, especially if you
> > > assign connection stats to it.
> >
> > ISTM we should just do this fairly obvious change. Given that we already
> > transport commit / rollback / IO stats, I don't see why the connection stats
> > change anything to a meaningful degree. I'm fairly baffled why that's not
> > the
> > obvious thing to do for v14.
>
> Here's how I think that would look like.
Thank you!
> While writing up this draft, I found
> two more issues:
>
> - On windows / 32 bit systems, the session time would overflow if idle for
> longer than ~4300s. long is only 32 bit. Easy to fix obviously.
Oops, yes. Thanks for spotting that.
> - Right now walsenders, including database connected walsenders, are not
> reported in connection stats. That doesn't seem quite right to me.
I think that walsenders not only use a different protocol, but often have
session characteristics quite different from normal backends.
For example, they are always "active", even when they are doing nothing.
Therefore, I think it is confusing to report them together with normal
database sessions.
If at all, walsender statistics should be reported separately.
> In the patch I made the message for connecting an explicitly reported message,
> that seems cleaner, because it then happens at a clearly defined point. I
> didn't do the same for disconnecting, but perhaps that would be better? Then
> we could get rid of the whole pgStatSessionEndCause variable.
I see your point, but I am not certain if it is worth adding yet another message
for a small thing like that. I have no strong opinion on that though.
Reading your patch, I am still confused about the following:
There are potentially several calls to "pgstat_send_tabstat" in
"pgstat_report_stat".
It seems to me that if it were called more than once, session statistics would
be reported and counted several times, which would be wrong.
Yours,
Laurenz Albe
