On Wed, Jan 15, 2025 at 08:30:20AM +0000, Bertrand Drouvot wrote: > On Wed, Jan 15, 2025 at 11:03:54AM +0300, Nazir Bilal Yavuz wrote: >> With this commit it may not be possible to count IOs in the critical >> sections. I think the problem happens only if the local >> PgStat_BackendPending entry is being created for the first time for >> this backend in the critical section. > > Yeah, I encountered the exact same thing and mentioned it in [1] (see R1.). > > In [1] I did propose to use a new PendingBackendWalStats variable to "bypass" > the > pgstat_prep_backend_pending() usage. > > Michael mentioned in [2] that is not really consistent with the rest (what I > agree with) and that "we should rethink a bit the way pending entries are > retrieved". I did not think about it yet but that might be the way to > go, thoughts? > > [1]: > https://www.postgresql.org/message-id/Z3zqc4o09dM/Ezyz%40ip-10-97-1-34.eu-west-3.compute.internal > [2]: https://www.postgresql.org/message-id/Z4dRlNuhSQ3hPPv2%40paquier.xyz
My problem is that this is not only related to backend stats, but to all variable-numbered stats kinds that require this behavior. One other case where I've seen this as being an issue is injection point stats, for example, while enabling these stats for all callbacks and some of them are run in critical sections. A generic solution to the problem would be to allow pgStatPendingContext to have MemoryContextAllowInCriticalSection() set temporarily for the allocation of the pending data. Perhaps not for all stats kinds, just for these where we're OK with the potential memory footprint so we could have a flag in PgStat_KindInfo. Giving to all stats kinds the responsibility to allocate a pending entry beforehand outside a critical section is another option, but that means going through different tweaks for all stats kinds that require them. Andres, do you have any thoughts about that? Not sure what would be your view on this matter. MemoryContextAllowInCriticalSection() for stats kinds that allow the case would be OK for me I guess, we should not be talking about a lot of memory. -- Michael
signature.asc
Description: PGP signature
