Hi,
On 2022-08-01 13:22:35 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2022-08-01 12:43:23 -0400, Tom Lane wrote:
> >> Yeah. I think we'd need to get rid of the "bool force" argument
> >> of pgstat_report_stat, and instead have it manage things internally
> >> based on understanding whether the current process uses a reporting
> >> timeout timer or not (if not, always send the report right away).
>
> > It's not as simple as looking at the backend type, I think. We'd not want to
> > enable a timer in the commit-and-chain context, even in a normal backend -
> > there's no chance we'll go idle.
>
> No, but that's not the point. During CommitTransaction we should check
> to see if it's more than X amount of time since our last stats report,
> and if so send a new one. No timer interaction involved there. When
> we go idle, if not inside a transaction and a report needs to be sent,
> then compute the wait time till it should be sent and set a timer for
> that.
I don't quite see how that'd help us to get rid of the force argument -
somehow pgstat_report_stat() needs to know that it it should "block" reporting
stats. Which we e.g. want when the idle timer has elapsed.
> > I wonder if it was the wrong call to use timers for IdleSessionTimeout,
> > IdleInTransactionSessionTimeout and pgstats. We always use nonblocking socket
> > IO these days, so perhaps we should instead just compute a relevant timeout
> > for the WaitEventSetWait() call?
>
> Meh. I think that that would end up with duplicative logic and
> duplicative gettimeofday calls, unless your idea is to get rid of
> the timeout.c facilities altogether.
I suspect it'd reduce the number of gettimeofday calls, but yes, the
duplication wouldn't be nice.
I guess there's a chance we could get rid of most of the timeout.c stuff, but
it'd need changes in a fair number of places... Don't think it's worth doing
atm.
Greetings,
Andres Freund