Hello Aleksander, > I'm not sure about what undefined behaviour could harm this badly. > > You are right that in practice nothing wrong will (probably) happen on > x86/x64 architecture with (most?) modern C compilers. This is not true in > the general case though. It's up to the compiler to decide how reading the > bufHdr->tag is going to be actually implemented. This can be one assembly > instruction or several instructions. This reading can be optimized-out if > the compiler believes the required value is already in the register, etc. > Since the result will be different depending on the assembly code used this > is an undefined behaviour and we can't use code like this. >
Got it. Thanks for explaining. > However I have somewhat mixed feelings about avg_usagecount. Generally > AVG() is a relatively useless methric for monitoring. What if the user > wants MIN(), MAX() or let's say a 99th percentile? I suggest splitting it > into usagecount_min, usagecount_max and usagecount_sum. AVG() can be > derived as usercount_sum / used_buffers. > Won't be usagecount_max almost always 5 as "BM_MAX_USAGE_COUNT" set to 5 in buf_internals.h? I'm not sure about how much usagecount_min would add either. A usagecount is always an integer between 0 and 5, it's not something unbounded. I think the 99th percentile would be much better than average if strong outlier values could occur. But in this case, I feel like an average value would be sufficiently useful as well. usagecount_sum would actually be useful since average can be derived from it. If you think that the sum of usagecounts has a meaning just by itself, it makes sense to include it. Otherwise, wouldn't showing directly averaged value be more useful? > Also I suggest changing the names of the columns in order to make them > consistent with the rest of the system. If you consider pg_stat_activity > and family [1] you will notice that the columns are named > (entity)_(property), e.g. backend_xid, backend_type, client_addr, etc. So > instead of used_buffers and unused_buffers the naming should be > buffers_used and buffers_unused. > > [1]: https://www.postgresql.org/docs/current/monitoring-stats.html > You're right. I will change the names accordingly. Thanks. Regards, Melih
