Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

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. 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.
- Right now walsenders, including database connected walsenders, are not
  reported in connection stats. That doesn't seem quite right to me.

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.

Greetings,

Andres Freund