Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

Hi,

On Mon, Jan 24, 2022 at 08:16:06PM +0300, Anton A. Melnikov wrote:
> Hi, Andrey!
>
> I've checked the 5th version of the patch and there are some remarks.
>
> >I've created a new view named pg_stat_statements_aux. But for now both
> >views are using the same function pg_stat_statements which returns all
> >fields. It seems reasonable to me - if sampling solution will need all
> >values it can query the function.
>
> Agreed, it might be useful in some cases.
>
> >But it seems "stats_reset" term is not quite correct in this case. The
> >"stats_since" field holds the timestamp of hashtable entry, but not the
> >reset time. The same applies to aux_stats_since - for new statement it
> >holds its entry time, but in case of reset it will hold the reset time.
>
> Thanks for the clarification. Indeed if we mean the word 'reset' as the
> removal of all the hashtable entries during pg_stat_statements_reset() then
> we shouldn't use it for the first occurrence timestamp in the struct
> pgssEntry.
> So with the stats_since field everything is clear.
> On the other hand aux_stats_since field can be updated for two reasons:
> 1) The same as for stats_since due to first occurrence of entry in the
> hashtable. And it will be equal to stats_since timestamp in that case.
> 2) Due to an external reset from an upper level sampler.
> I think it's not very important how to name this field, but it would be
> better to mention both these reasons in the comment.

Are those 4 new counters really worth it?

The min/max were added to make pg_stat_statements a bit more useful if you
have nothing else using that extension. But once you setup a tool that
snapshots the metrics regularly, do you really need to know e.g. "what was the
maximum execution time in the last 3 years", which will typically be what
people will retrieve from the non-aux min/max counters, rather than simply
using your additional tool for better and more precise information?