Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements - Mailing list pgsql-hackers

From Andrei Zubkov
Subject Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements
Date
Msg-id 74b0960113836e52eb6dea94c4c6b0a49bc9e4c4.camel@moonset.ru
Whole thread Raw
In response to Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements  (Julien Rouhaud <rjuju123@gmail.com>)
Responses Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements  (Andres Freund <andres@anarazel.de>)
Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements  (Julien Rouhaud <rjuju123@gmail.com>)
List pgsql-hackers
Hi,

Thank you, Greg

On Fri, 2022-04-01 at 11:38 -0400, Greg Stark wrote:
> [13:19:51.544] pg_stat_statements.c: In function ‘entry_reset’:
> [13:19:51.544] pg_stat_statements.c:2598:32: error:
> ‘minmax_stats_reset’ may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
> [13:19:51.544] 2598 | entry->minmax_stats_since = minmax_stats_reset;
> [13:19:51.544] | ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
>

I was afraid of such warning can appear..

On Sat, 2022-04-02 at 00:13 +0800, Julien Rouhaud wrote:
> I guess that pg_stat_statement_reset() is so
> expensive that an extra gettimeofday() wouldn't change much. 

Agreed

> Otherwise
> initializing to NULL should be enough.

Julien, I would prefer an extra GetCurrentTimestamp(). So, I've opted
to use the common unconditional

stats_reset = GetCurrentTimestamp();

for an entire entry_reset function due to the following:

1. It will be uniform for stats_reset and minmax_stats_reset
2. As you mentioned, it wouldn't change a much
3. The most common way to use this function is to reset all statements
meaning that GetCurrentTimestamp() will be called anyway to update the
value of stats_reset field in pg_stat_statements_info view
4. Actually I would like that pg_stat_statements_reset function was
able to return the value of stats_reset as its result. This could give
to the sampling solutions the ability to check if the last reset (of
any type) was performed by this solution or any other reset was
performed by someone else. It seems valuable to me, but it changes the
result type of the pg_stat_statements_reset() function, so I don't know
if we can do that right now.

v8 attached
--
regards, Andrei

Attachment

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Can we automatically add elapsed times to tap test log?
Next
From: Andres Freund
Date:
Subject: Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements