Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

Hi,

On 2021-09-08 06:11:56 +0200, Laurenz Albe wrote:
> I have gone over your patch and made the following changes:
>
> - cache the last report time in a static variable pgLastSessionReportTime
> - add a comment to explain why we only track normal backends
> - pgindent
> - an attempt at a commit message

Thanks!

I pushed this. The only substantive change I made is that I moved the
MyBackendType == B_BACKEND check into a new pgstat_should_report_connstat(),
and called that from pgstat_report_connect() and
pgstat_report_disconnect(). Otherwise we'd report connect/disconnect for
walsenders, but not the session time, which seems even weirder.

Closed the open item.

I'm not convinced that ignorign walsenders is a good policy decision, but I
think that's a fair judgement call, and we can evolve that in future
releases...

One thing that'd be nice to have is a few regression tests in stats.sql. Of
course we couldn't test precise counts, but just verifying that some of the
counters increase seems like it would provide a decent increase in "actual"
coverage. One of the more painful things in working on the shared memory stats
patch is that nearly all stats features have no tests, making it very easy to
silently break things.

Greetings,

Andres Freund