pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18046: stats collection behaviour change is affecting the usability of information.
10+ messages / 6 participants
[nested] [flat]
* BUG #18046: stats collection behaviour change is affecting the usability of information.
@ 2023-08-01 16:32 PG Bug reporting form <noreply@postgresql.org>
2023-08-04 11:22 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Hamid Akhtar <hamid.akhtar@percona.com>
0 siblings, 1 reply; 10+ messages in thread
From: PG Bug reporting form @ 2023-08-01 16:32 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: jobinau@gmail.com
The following bug has been logged on the website:
Bug reference: 18046
Logged by: Jobin Augustine
Email address: jobinau@gmail.com
PostgreSQL version: 15.3
Operating system: CentOS/RHEL
Description:
After stats collection changes in PG 15, The behaviour stats_reset
information is changed. which is adversely affecting the usability of data
presented in the view.
```
select stats_reset from pg_stat_database;
```
Before PG15, stats_reset was always populated so that the user knows the
cumulative values presented are of how many days.
But unfortunately, PG 15 keeps it null unless there is an explicit reset
(pg_stat_reset). The value goes again null if there is a reset due to a
crash.
So on a regular system, the User can't understand the cumulative values
presented in the view.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18046: stats collection behaviour change is affecting the usability of information.
2023-08-01 16:32 BUG #18046: stats collection behaviour change is affecting the usability of information. PG Bug reporting form <noreply@postgresql.org>
@ 2023-08-04 11:22 ` Hamid Akhtar <hamid.akhtar@percona.com>
2023-08-04 17:10 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Jobin Augustine <jobin.augustine@percona.com>
0 siblings, 1 reply; 10+ messages in thread
From: Hamid Akhtar @ 2023-08-04 11:22 UTC (permalink / raw)
To: jobinau@gmail.com; pgsql-bugs@lists.postgresql.org
On Tue, 1 Aug 2023 at 22:01, PG Bug reporting form <noreply@postgresql.org>
wrote:
> The following bug has been logged on the website:
>
> Bug reference: 18046
> Logged by: Jobin Augustine
> Email address: jobinau@gmail.com
> PostgreSQL version: 15.3
> Operating system: CentOS/RHEL
> Description:
>
> After stats collection changes in PG 15, The behaviour stats_reset
> information is changed. which is adversely affecting the usability of data
> presented in the view.
> ```
> select stats_reset from pg_stat_database;
> ```
> Before PG15, stats_reset was always populated so that the user knows the
> cumulative values presented are of how many days.
> But unfortunately, PG 15 keeps it null unless there is an explicit reset
> (pg_stat_reset). The value goes again null if there is a reset due to a
> crash.
> So on a regular system, the User can't understand the cumulative values
> presented in the view.
>
>
Thank you for the bug report Jobin.
IMHO, this is a valid concern. As per the documentation, the "stats_reset"
column tracks the last time the stats were reset. There is no mention of
this being timestamp for manual reset only.
Internally, the server calls pgstat_reset_after_failure whenever it cannot
find the stats file or if the server is recovering from a crash. In case of
a crash, the stats may no longer be valid. Therefore, internally the stats
are dropped, and a new file is written.
Attach is a fix for PG16 and PG15 that resolves this issue. It ensures that
when the database stats are being written to disk and the stats_reset is
not set, it adds the current timestamp to it. Since a new file is written
at initdb and when the server is recovering from a crash, this works as
expected.
Attachments:
pg16_stat_reset_fix.patch (application/octet-stream, 905B) download
diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c
index 7149f22..7c260a4 100644
--- a/src/backend/utils/activity/pgstat_database.c
+++ b/src/backend/utils/activity/pgstat_database.c
@@ -363,6 +363,7 @@ pgstat_database_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
{
PgStatShared_Database *sharedent;
PgStat_StatDBEntry *pendingent;
+ TimestampTz ts = GetCurrentTimestamp();
pendingent = (PgStat_StatDBEntry *) entry_ref->pending;
sharedent = (PgStatShared_Database *) entry_ref->shared_stats;
@@ -370,6 +371,9 @@ pgstat_database_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
if (!pgstat_lock_entry(entry_ref, nowait))
return false;
+ if ((sharedent)->stats.stat_reset_timestamp == 0)
+ (sharedent)->stats.stat_reset_timestamp = ts;
+
#define PGSTAT_ACCUM_DBCOUNT(item) \
(sharedent)->stats.item += (pendingent)->item
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18046: stats collection behaviour change is affecting the usability of information.
2023-08-01 16:32 BUG #18046: stats collection behaviour change is affecting the usability of information. PG Bug reporting form <noreply@postgresql.org>
2023-08-04 11:22 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Hamid Akhtar <hamid.akhtar@percona.com>
@ 2023-08-04 17:10 ` Jobin Augustine <jobin.augustine@percona.com>
2023-09-06 16:57 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 10+ messages in thread
From: Jobin Augustine @ 2023-08-04 17:10 UTC (permalink / raw)
To: Hamid Akhtar <hamid.akhtar@percona.com>; +Cc: jobinau@gmail.com; pgsql-bugs@lists.postgresql.org
Thank you Hamid for working on this and coming with a fix.
On Fri, Aug 4, 2023 at 4:53 PM Hamid Akhtar <hamid.akhtar@percona.com>
wrote:
>
> Thank you for the bug report Jobin.
>
> IMHO, this is a valid concern. As per the documentation, the "stats_reset"
> column tracks the last time the stats were reset. There is no mention of
> this being timestamp for manual reset only.
>
> Without this base info, users don't have the option to understand the
cumulative statistics in the stats view
>
> Attach is a fix for PG16 and PG15 that resolves this issue. It ensures
> that when the database stats are being written to disk and the stats_reset
> is not set, it adds the current timestamp to it. Since a new file is
> written at initdb and when the server is recovering from a crash, this
> works as expected.
>
>
I can confirm that this patch fixes the problem.
I could find simple steps to reproduce the original problem independently.
Step 1 : Create a new database
CREATE DATABASE db1;
Step 2. Create a table in the database
\c db1
CREATE TABLE t1 (id INT);
Step 3. Check the timestamp of the start of database-level statistics
db1=# SELECT datname,stats_reset FROM pg_stat_database;
Expected behaviour(works in all versions upto and including PostgreSQL 14)
datname | stats_reset
-----------+-------------------------------
| 2023-08-02 06:41:15.777135+00
postgres | 2023-08-02 06:41:15.777108+00
template1 |
template0 |
db1 | 2023-08-02 11:02:54.954363+00
(5 rows)
The problem in PostgreSQL 15 and above
datname | stats_reset
-----------+------------------------------
|
postgres |
db1 |
template1 |
template0 |
(5 rows)
Once again, Thank you for the fix.
Jobin Augustine.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18046: stats collection behaviour change is affecting the usability of information.
2023-08-01 16:32 BUG #18046: stats collection behaviour change is affecting the usability of information. PG Bug reporting form <noreply@postgresql.org>
2023-08-04 11:22 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Hamid Akhtar <hamid.akhtar@percona.com>
2023-08-04 17:10 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Jobin Augustine <jobin.augustine@percona.com>
@ 2023-09-06 16:57 ` Bruce Momjian <bruce@momjian.us>
2023-09-06 17:06 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Bruce Momjian <bruce@momjian.us>
2023-09-07 05:12 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 2 replies; 10+ messages in thread
From: Bruce Momjian @ 2023-09-06 16:57 UTC (permalink / raw)
To: Jobin Augustine <jobin.augustine@percona.com>; +Cc: Hamid Akhtar <hamid.akhtar@percona.com>; jobinau@gmail.com, pgsql-bugs@lists.postgresql.org, Andres Freund <andres@anarazel.de>
Andres, can you comment on this thread? I see you had a commit to PG
15 in this area:
commit 5cd1c40b3c
Author: Andres Freund <andres@anarazel.de>
Date: Thu Apr 14 17:40:25 2022 -0700
pgstat: set timestamps of fixed-numbered stats after a crash.
When not loading stats at startup (i.e. pgstat_discard_stats() getting
called), reset timestamps of fixed numbered stats would be left at
0. Oversight in 5891c7a8ed8.
Instead use pgstat_reset_after_failure() and add tests verifying that
fixed-numbered reset timestamps are set appropriately.
Reported-By: "David G. Johnston" <david.g.johnston@gmail.com>
Discussion: https://postgr.es/m/CAKFQuwamFuaQHKdhcMt4Gbw5+Hca2UE741B8gOOXoA=TtAd2Yw@mail.gmail.com
Thanks.
---------------------------------------------------------------------------
On Fri, Aug 4, 2023 at 10:40:46PM +0530, Jobin Augustine wrote:
> Thank you Hamid for working on this and coming with a fix.
>
> On Fri, Aug 4, 2023 at 4:53 PM Hamid Akhtar <hamid.akhtar@percona.com> wrote:
>
>
> Thank you for the bug report Jobin.
>
> IMHO, this is a valid concern. As per the documentation, the "stats_reset"
> column tracks the last time the stats were reset. There is no mention of
> this being timestamp for manual reset only.
>
>
> Without this base info, users don't have the option to understand the
> cumulative statistics in the stats view
>
>
>
> Attach is a fix for PG16 and PG15 that resolves this issue. It ensures that
> when the database stats are being written to disk and the stats_reset is
> not set, it adds the current timestamp to it. Since a new file is written
> at initdb and when the server is recovering from a crash, this works as
> expected.
>
>
>
> I can confirm that this patch fixes the problem.
> I could find simple steps to reproduce the original problem independently.
>
>
> Step 1 : Create a new database
> CREATE DATABASE db1;
>
> Step 2. Create a table in the database
> \c db1
> CREATE TABLE t1 (id INT);
>
> Step 3. Check the timestamp of the start of database-level statistics
> db1=# SELECT datname,stats_reset FROM pg_stat_database;
>
> Expected behaviour(works in all versions upto and including PostgreSQL 14)
>
> datname | stats_reset
> -----------+-------------------------------
> | 2023-08-02 06:41:15.777135+00
> postgres | 2023-08-02 06:41:15.777108+00
> template1 |
> template0 |
> db1 | 2023-08-02 11:02:54.954363+00
> (5 rows)
>
>
> The problem in PostgreSQL 15 and above
>
> datname | stats_reset
> -----------+------------------------------
> |
> postgres |
> db1 |
> template1 |
> template0 |
> (5 rows)
>
>
> Once again, Thank you for the fix.
>
> Jobin Augustine.
>
>
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18046: stats collection behaviour change is affecting the usability of information.
2023-08-01 16:32 BUG #18046: stats collection behaviour change is affecting the usability of information. PG Bug reporting form <noreply@postgresql.org>
2023-08-04 11:22 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Hamid Akhtar <hamid.akhtar@percona.com>
2023-08-04 17:10 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Jobin Augustine <jobin.augustine@percona.com>
2023-09-06 16:57 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Bruce Momjian <bruce@momjian.us>
@ 2023-09-06 17:06 ` Bruce Momjian <bruce@momjian.us>
2023-09-13 04:20 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Andres Freund <andres@anarazel.de>
2023-09-14 03:30 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Andres Freund <andres@anarazel.de>
1 sibling, 2 replies; 10+ messages in thread
From: Bruce Momjian @ 2023-09-06 17:06 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Hamid Akhtar <hamid.akhtar@percona.com>; jobinau@gmail.com, pgsql-bugs@lists.postgresql.org, Jobin Augustine <jobin.augustine@percona.com>
(This email has Andres properly in the TO field.)
Andres, can you comment on this thread? I see you had a commit to PG
15 in this area:
commit 5cd1c40b3c
Author: Andres Freund <andres@anarazel.de>
Date: Thu Apr 14 17:40:25 2022 -0700
pgstat: set timestamps of fixed-numbered stats after a crash.
When not loading stats at startup (i.e. pgstat_discard_stats() getting
called), reset timestamps of fixed numbered stats would be left at
0. Oversight in 5891c7a8ed8.
Instead use pgstat_reset_after_failure() and add tests verifying that
fixed-numbered reset timestamps are set appropriately.
Reported-By: "David G. Johnston" <david.g.johnston@gmail.com>
Discussion: https://postgr.es/m/CAKFQuwamFuaQHKdhcMt4Gbw5+Hca2UE741B8gOOXoA=TtAd2Yw@mail.gmail.com
Thanks.
---------------------------------------------------------------------------
On Fri, Aug 4, 2023 at 10:40:46PM +0530, Jobin Augustine wrote:
> Thank you Hamid for working on this and coming with a fix.
>
> On Fri, Aug 4, 2023 at 4:53 PM Hamid Akhtar <hamid.akhtar@percona.com> wrote:
>
>
> Thank you for the bug report Jobin.
>
> IMHO, this is a valid concern. As per the documentation, the "stats_reset"
> column tracks the last time the stats were reset. There is no mention of
> this being timestamp for manual reset only.
>
>
> Without this base info, users don't have the option to understand the
> cumulative statistics in the stats view
>
>
>
> Attach is a fix for PG16 and PG15 that resolves this issue. It ensures that
> when the database stats are being written to disk and the stats_reset is
> not set, it adds the current timestamp to it. Since a new file is written
> at initdb and when the server is recovering from a crash, this works as
> expected.
>
>
>
> I can confirm that this patch fixes the problem.
> I could find simple steps to reproduce the original problem independently.
>
>
> Step 1 : Create a new database
> CREATE DATABASE db1;
>
> Step 2. Create a table in the database
> \c db1
> CREATE TABLE t1 (id INT);
>
> Step 3. Check the timestamp of the start of database-level statistics
> db1=# SELECT datname,stats_reset FROM pg_stat_database;
>
> Expected behaviour(works in all versions upto and including PostgreSQL 14)
>
> datname | stats_reset
> -----------+-------------------------------
> | 2023-08-02 06:41:15.777135+00
> postgres | 2023-08-02 06:41:15.777108+00
> template1 |
> template0 |
> db1 | 2023-08-02 11:02:54.954363+00
> (5 rows)
>
>
> The problem in PostgreSQL 15 and above
>
> datname | stats_reset
> -----------+------------------------------
> |
> postgres |
> db1 |
> template1 |
> template0 |
> (5 rows)
>
>
> Once again, Thank you for the fix.
>
> Jobin Augustine.
>
>
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18046: stats collection behaviour change is affecting the usability of information.
2023-08-01 16:32 BUG #18046: stats collection behaviour change is affecting the usability of information. PG Bug reporting form <noreply@postgresql.org>
2023-08-04 11:22 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Hamid Akhtar <hamid.akhtar@percona.com>
2023-08-04 17:10 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Jobin Augustine <jobin.augustine@percona.com>
2023-09-06 16:57 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Bruce Momjian <bruce@momjian.us>
2023-09-06 17:06 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Bruce Momjian <bruce@momjian.us>
@ 2023-09-13 04:20 ` Andres Freund <andres@anarazel.de>
1 sibling, 0 replies; 10+ messages in thread
From: Andres Freund @ 2023-09-13 04:20 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: Hamid Akhtar <hamid.akhtar@percona.com>; jobinau@gmail.com, pgsql-bugs@lists.postgresql.org, Jobin Augustine <jobin.augustine@percona.com>
Hi,
On 2023-09-06 13:06:35 -0400, Bruce Momjian wrote:
> (This email has Andres properly in the TO field.)
>
> Andres, can you comment on this thread? I see you had a commit to PG
> 15 in this area:
I'll try to look at it soon. I'm only now catching up on email after being on
vacation.
Regards,
Andres
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18046: stats collection behaviour change is affecting the usability of information.
2023-08-01 16:32 BUG #18046: stats collection behaviour change is affecting the usability of information. PG Bug reporting form <noreply@postgresql.org>
2023-08-04 11:22 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Hamid Akhtar <hamid.akhtar@percona.com>
2023-08-04 17:10 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Jobin Augustine <jobin.augustine@percona.com>
2023-09-06 16:57 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Bruce Momjian <bruce@momjian.us>
2023-09-06 17:06 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Bruce Momjian <bruce@momjian.us>
@ 2023-09-14 03:30 ` Andres Freund <andres@anarazel.de>
2023-09-19 08:36 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Jobin Augustine <jobin.augustine@percona.com>
1 sibling, 1 reply; 10+ messages in thread
From: Andres Freund @ 2023-09-14 03:30 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: Hamid Akhtar <hamid.akhtar@percona.com>; jobinau@gmail.com, pgsql-bugs@lists.postgresql.org, Jobin Augustine <jobin.augustine@percona.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Hi,
On 2023-09-06 13:06:35 -0400, Bruce Momjian wrote:
> (This email has Andres properly in the TO field.)
>
> Andres, can you comment on this thread? I see you had a commit to PG
> 15 in this area:
>
> commit 5cd1c40b3c
> Author: Andres Freund <andres@anarazel.de>
> Date: Thu Apr 14 17:40:25 2022 -0700
>
> pgstat: set timestamps of fixed-numbered stats after a crash.
>
> When not loading stats at startup (i.e. pgstat_discard_stats() getting
> called), reset timestamps of fixed numbered stats would be left at
> 0. Oversight in 5891c7a8ed8.
>
> Instead use pgstat_reset_after_failure() and add tests verifying that
> fixed-numbered reset timestamps are set appropriately.
>
> Reported-By: "David G. Johnston" <david.g.johnston@gmail.com>
> Discussion: https://postgr.es/m/CAKFQuwamFuaQHKdhcMt4Gbw5+Hca2UE741B8gOOXoA=TtAd2Yw@mail.gmail.com
>
> Thanks.
I don't think that commit is relevant, but I still am to blame :)
> On Fri, Aug 4, 2023 at 10:40:46PM +0530, Jobin Augustine wrote:
> > Thank you Hamid for working on this and coming with a fix.
> > Attach is a fix for PG16 and PG15 that resolves this issue. It ensures that
> > when the database stats are being written to disk and the stats_reset is
> > not set, it adds the current timestamp to it. Since a new file is written
> > at initdb and when the server is recovering from a crash, this works as
> > expected.
I don't think that's the right approach. This way we'll end up with a stats
reset time that can be *after* other stats have already accumulated. Which
doesn't seem great.
I think Horiguchi-san's approach is closer to what we would want.
Unfortunately, the reset time for other variable-numbered stats entries
historically has behaved differently than what you're
desiring. E.g. pg_stat_replication_slots.stats_reset isn't set unless you have
reset the stats.
I'm ok with changing the semantics to one behaviour, but I'm a bit hesitant
whacking this around further in a minor release...
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18046: stats collection behaviour change is affecting the usability of information.
2023-08-01 16:32 BUG #18046: stats collection behaviour change is affecting the usability of information. PG Bug reporting form <noreply@postgresql.org>
2023-08-04 11:22 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Hamid Akhtar <hamid.akhtar@percona.com>
2023-08-04 17:10 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Jobin Augustine <jobin.augustine@percona.com>
2023-09-06 16:57 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Bruce Momjian <bruce@momjian.us>
2023-09-06 17:06 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Bruce Momjian <bruce@momjian.us>
2023-09-14 03:30 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Andres Freund <andres@anarazel.de>
@ 2023-09-19 08:36 ` Jobin Augustine <jobin.augustine@percona.com>
0 siblings, 0 replies; 10+ messages in thread
From: Jobin Augustine @ 2023-09-19 08:36 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; Bruce Momjian <bruce@momjian.us>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; Hamid Akhtar <hamid.akhtar@percona.com>; +Cc: jobinau@gmail.com; pgsql-bugs@lists.postgresql.org
> I think Horiguchi-san's approach is closer to what we would want.
>
> Great.
>
> Unfortunately, the reset time for other variable-numbered stats entries
> historically has behaved differently than what you're
> desiring. E.g. pg_stat_replication_slots.stats_reset isn't set unless you
> have
> reset the stats.
>
Conceptually, that's correct. But the Impact on users are different.
SQL statements around pg_stat_database are something every user (DBA) has
in their arsenal for essential checks and troubleshooting.
The impact is big when their queries stopped working/giving wrong results
from PostgeSQL 15 onwards.
> I'm ok with changing the semantics to one behaviour,
Great. I believe everyone will appreciate a consistent behaviour
> but I'm a bit hesitant
> whacking this around further in a minor release...
>
>
Since this sudden undocumented behaviour change caught everyone by
surprise,
and case this is a too big change for a "bug-fix", is it possible to have
a temporary fix for a minor version, fixing pg_stat_database alone?
> Greetings,
> Andres Freund
>
Regards,
Jobin Augustine
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18046: stats collection behaviour change is affecting the usability of information.
2023-08-01 16:32 BUG #18046: stats collection behaviour change is affecting the usability of information. PG Bug reporting form <noreply@postgresql.org>
2023-08-04 11:22 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Hamid Akhtar <hamid.akhtar@percona.com>
2023-08-04 17:10 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Jobin Augustine <jobin.augustine@percona.com>
2023-09-06 16:57 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Bruce Momjian <bruce@momjian.us>
@ 2023-09-07 05:12 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2023-09-07 05:16 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Kyotaro Horiguchi <horikyota.ntt@gmail.com>
1 sibling, 1 reply; 10+ messages in thread
From: Kyotaro Horiguchi @ 2023-09-07 05:12 UTC (permalink / raw)
To: bruce@momjian.us; +Cc: jobin.augustine@percona.com; hamid.akhtar@percona.com; jobinau@gmail.com; pgsql-bugs@lists.postgresql.org; andres@anarazel.de
At Wed, 6 Sep 2023 12:57:01 -0400, Bruce Momjian <bruce@momjian.us> wrote in
> > Expected behaviour(works in all versions upto and including PostgreSQL 14)
> >
> > datname | stats_reset
> > -----------+-------------------------------
> > | 2023-08-02 06:41:15.777135+00
> > postgres | 2023-08-02 06:41:15.777108+00
> > template1 |
> > template0 |
> > db1 | 2023-08-02 11:02:54.954363+00
> > (5 rows)
> >
> >
> > The problem in PostgreSQL 15 and above
> >
> > datname | stats_reset
> > -----------+------------------------------
> > |
> > postgres |
> > db1 |
> > template1 |
> > template0 |
> > (5 rows)
I agree it's not ideal to reset timestamps only for fixed-numbered
stats. In 13, the timestamp is set to the creation time. There's no
practical issue with doing it that way for now, I think it's
generallynot preferable.
We could use the transaction start time as a sufficient approximation
for the timestamp in fast paths. It might not align with with the
reset time set by pg_stat_reset(), but that shouldn't be a
problem. pgstat_restore_stats runs outside of a transaction, but
GetCurrentTimestamp() is already being used there. The reason for
defining a new function is to improve robustness, although it might
not be necessary.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
pgstat_set_reset_time_after_crash.patch (text/x-patch, 3.2K) download
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index d743fc0b28..b86c87bbe0 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -395,12 +395,27 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
},
};
-
/* ------------------------------------------------------------
* Functions managing the state of the stats system for all backends.
* ------------------------------------------------------------
*/
+/*
+ * Return the timestamp when called the first time. This is intended to be used
+ * to obatin the reset timestamp when a shared entry is created.
+ */
+TimestampTz
+pgstat_get_reset_timestamp(void)
+{
+ TimestampTz ts = GetCurrentTransactionStartTimestamp();
+
+ /* fall back to GetCurrentTimestamp if not available */
+ if (ts > 0)
+ return ts;
+
+ return GetCurrentTimestamp();
+}
+
/*
* Read on-disk stats into memory at server start.
*
@@ -1683,7 +1698,7 @@ error:
static void
pgstat_reset_after_failure(void)
{
- TimestampTz ts = GetCurrentTimestamp();
+ TimestampTz ts = pgstat_get_reset_timestamp();
/* reset fixed-numbered stats */
for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++)
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index d1149adf70..ff4d03150e 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -258,7 +258,9 @@ PgStatShared_Common *
pgstat_init_entry(PgStat_Kind kind,
PgStatShared_HashEntry *shhashent)
{
- /* Create new stats entry. */
+ const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
+
+ /* Create new stats entry. */
dsa_pointer chunk;
PgStatShared_Common *shheader;
@@ -278,6 +280,9 @@ pgstat_init_entry(PgStat_Kind kind,
/* Link the new entry from the hash entry. */
shhashent->body = chunk;
+ if (kind_info->reset_timestamp_cb)
+ kind_info->reset_timestamp_cb(shheader, pgstat_get_reset_timestamp());
+
LWLockInitialize(&shheader->lock, LWTRANCHE_PGSTATS_DATA);
return shheader;
@@ -286,6 +291,7 @@ pgstat_init_entry(PgStat_Kind kind,
static PgStatShared_Common *
pgstat_reinit_entry(PgStat_Kind kind, PgStatShared_HashEntry *shhashent)
{
+ const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
PgStatShared_Common *shheader;
shheader = dsa_get_address(pgStatLocal.dsa, shhashent->body);
@@ -299,6 +305,9 @@ pgstat_reinit_entry(PgStat_Kind kind, PgStatShared_HashEntry *shhashent)
memset(pgstat_get_entry_data(kind, shheader), 0,
pgstat_get_entry_len(kind));
+ if (kind_info->reset_timestamp_cb)
+ kind_info->reset_timestamp_cb(shheader, pgstat_get_reset_timestamp());
+
return shheader;
}
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 57a2c0866a..df2323c5bc 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -464,6 +464,7 @@ extern Size StatsShmemSize(void);
extern void StatsShmemInit(void);
/* Functions called during server startup / shutdown */
+extern TimestampTz pgstat_get_reset_timestamp(void);
extern void pgstat_restore_stats(void);
extern void pgstat_discard_stats(void);
extern void pgstat_before_server_shutdown(int code, Datum arg);
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18046: stats collection behaviour change is affecting the usability of information.
2023-08-01 16:32 BUG #18046: stats collection behaviour change is affecting the usability of information. PG Bug reporting form <noreply@postgresql.org>
2023-08-04 11:22 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Hamid Akhtar <hamid.akhtar@percona.com>
2023-08-04 17:10 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Jobin Augustine <jobin.augustine@percona.com>
2023-09-06 16:57 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Bruce Momjian <bruce@momjian.us>
2023-09-07 05:12 ` Re: BUG #18046: stats collection behaviour change is affecting the usability of information. Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2023-09-07 05:16 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 0 replies; 10+ messages in thread
From: Kyotaro Horiguchi @ 2023-09-07 05:16 UTC (permalink / raw)
To: bruce@momjian.us; +Cc: jobin.augustine@percona.com; hamid.akhtar@percona.com; jobinau@gmail.com; pgsql-bugs@lists.postgresql.org; andres@anarazel.de
At Thu, 07 Sep 2023 14:12:32 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
> We could use the transaction start time as a sufficient approximation
> for the timestamp in fast paths. It might not align with with the
> reset time set by pg_stat_reset(), but that shouldn't be a
> problem. pgstat_restore_stats runs outside of a transaction, but
To be accurate, it's not "outside of a transaction", but "before any
transaction starts".
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2023-09-19 08:36 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-08-01 16:32 BUG #18046: stats collection behaviour change is affecting the usability of information. PG Bug reporting form <noreply@postgresql.org>
2023-08-04 11:22 ` Hamid Akhtar <hamid.akhtar@percona.com>
2023-08-04 17:10 ` Jobin Augustine <jobin.augustine@percona.com>
2023-09-06 16:57 ` Bruce Momjian <bruce@momjian.us>
2023-09-06 17:06 ` Bruce Momjian <bruce@momjian.us>
2023-09-13 04:20 ` Andres Freund <andres@anarazel.de>
2023-09-14 03:30 ` Andres Freund <andres@anarazel.de>
2023-09-19 08:36 ` Jobin Augustine <jobin.augustine@percona.com>
2023-09-07 05:12 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2023-09-07 05:16 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox