On 07.10.2021 15:31, Andrei Zubkov wrote: > There is an issue with this patch. It's main purpose is the ability to > calculate values of pg_stat_statements view > [...] > Does addition of resettable min/max metrics to the > pg_stat_statemets view seems reasonable here?
Hello, Andrey! I think it depends on what the slow top level sampler wants. Let define the current values in pg_stat_statements for some query as gmin and gmax. It seems to be a two main variants: 1) If top level sampler wants to know for some query the min and max values for the entire watch time, then existing gmin and gmax in pg_stat_statements are sufficient. The top level sampler can clarify top_min and top_max at every slow sample as follows: top_max = gmax > top_max ? gmax : top_max; top_min = gmin < top_min ? gmin : top_min; This should work regardless of whether there was a reset between samples or not. 2) The second case happens when the top level sampler wants to know the min and max values for sampling period. In that case i think one shouldn't not use gmin and gmax and especially reset them asynchronously from outside because its lifetime and sampling period are independent values and moreover someone else might need gmin and gmax as is. So i suppose that additional vars loc_min and loc_max is a right way to do it. If that, at every top sample one need to replace not clarify the new top values as follows: top_max = loc_max; loc_max = 0; top_min = loc_min; loc_min = INT_MAX; And one more thing, if there was a reset of stats between two samples, then i think it is the best to ignore the new values, since they were obtained for an incomplete period. This patch, thanks to the saved time stamp, makes possible to determine the presence of reset between samples and there should not be a problem to realize such algorithm. The patch is now applied normally, all my tests were successful. The only thing I could suggest to your notice is a small cosmetic edit to replace the numeric value in #define on line 1429 of pg_stat_statements.c by one of the constants defined above. Best regards, Anton Melnikov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
