Hi Melih,
> 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.
> In the attached patch, I added buffer header locks just before examining tag as follows
Many thanks for the updated patch! It looks better now.
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.
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--
Best regards,
Aleksander Alekseev
