Hi!
On 09.12.2024 11:03, Bertrand Drouvot wrote:
There is a missing space. I think that should be " at server..." or "...%llu ".
Thanks for pointing this out. In the other code the elog messages are all on
one line,
regardless of their length. Did the same in v3.
On 10.12.2024 09:42, Bertrand Drouvot wrote:
On Tue, Dec 10, 2024 at 09:54:36AM +0900, Michael Paquier wrote:
On Mon, Dec 09, 2024 at 08:03:54AM +0000, Bertrand Drouvot wrote:
Right. OTOH I think that could help the tap test added in da99fedf8c to not
rely on assert enabled build (the tap test could "simply" check for the
WARNING in the logfile instead).
That's true. Still, the coverage that we have is also enough for
assert builds, which is what the test is going to run with most of the
time anyway.
Yeah, that's fine by me and don't see the added value of the WARNING then.
Agreed that this WARNING has no additional value for testing purposes
at pgfarm or ci. Assert is better.
My logic was different.
It's clear that during normal server operation this code should be unreachable.
But we admit that in production deployments it can be executed in case of some
bug
that is still unknown to us. Now it is done in such a way that in this case
the server simply skip it and won't notice about it. And no one will know that
this happened.
But if there is a warning here, the information will remain in the server logs,
we can find out about it and we can try to reproduce similar behavior
in the testing environment and probably detect a hidden bug like in [1].
Thanks a lot for fixing this!
With the best regards,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
[1]
https://www.postgresql.org/message-id/56bf8ff9-dd8c-47b2-872a-748ede82af99%40postgrespro.ru
From 3ff955ba7674c78a162a3c0243b28c5004768e07 Mon Sep 17 00:00:00 2001
From: "Anton A. Melnikov" <a.melni...@postgrespro.ru>
Date: Sat, 7 Dec 2024 12:00:10 +0300
Subject: [PATCH] Add warning
---
src/backend/utils/activity/pgstat.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 7533dea6407..c1b5995f0ea 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -1665,7 +1665,13 @@ pgstat_write_statsfile(XLogRecPtr redo)
*/
Assert(!ps->dropped);
if (ps->dropped)
+ {
+ PgStat_HashKey key = ps->key;
+ elog(WARNING, "found non-deleted stats entry %u/%u/%llu at server shutdown",
+ key.kind, key.dboid,
+ (unsigned long long) key.objid);
continue;
+ }
/*
* This discards data related to custom stats kinds that are unknown
--
2.47.1
