Re: Support reset of Shared objects statistics in "pg_stat_reset" function

From: Mahendra Singh Thalor <mahi6run(at)gmail(dot)com>
To: Sadhuprasad Patro <b(dot)sadhu(at)gmail(dot)com>
Cc: Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Himanshu Upadhyaya <upadhyaya(dot)himanshu(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Support reset of Shared objects statistics in "pg_stat_reset" function
Date: 2021-08-10 17:02:14
Message-ID: CAKYtNAoeVztOdF4ggOYPcUKo61uHTuZ1oY-GeUYLwHuJTO82Lg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 10 Aug 2021 at 21:53, Sadhuprasad Patro <b(dot)sadhu(at)gmail(dot)com> wrote:
>
> > As of now, we are adding handling inside pg_stat_reset for shared
> > tables but I think we can add a new function with the name of
> > pg_stat_reset_shared_tables to reset stats for all the shared tables.
> > New function will give more clarity to the users also. We already have
> > a pg_stat_reset_shared(text) function for "archiver", "bgwriter", or
> > "wal".
> >
> > Thoughts?
>
> In my opinion, it is better to extend the functionality of
> "pg_stat_reset" call because a new function just to reset shared table
> data may not be needed. Where we already have a reset shared function
> "pg_stat_reset_shared" in place.
>
> All of applicable comments are implemented in the patch below:
>

Hi Sadhu,
I can see that you forgot to include "catalog.h" so I am getting below
warning:

> pgstat.c: In function ‘pgstat_recv_resetsinglecounter’:
> pgstat.c:5216:7: warning: implicit declaration of function
> ‘IsSharedRelation’; did you mean ‘InvalidRelation’?
> [-Wimplicit-function-declaration]
> if (!IsSharedRelation(msg->m_objectid))
> ^~~~~~~~~~~~~~~~
> InvalidRelation
>

1) Please add the .h file.
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -38,6 +38,7 @@
#include "access/transam.h"
#include "access/twophase_rmgr.h"
#include "access/xact.h"
+#include "catalog/catalog.h"
#include "catalog/partition.h"

2)
@@ -1442,6 +1443,10 @@ pgstat_reset_counters(void)
pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_RESETCOUNTER);
msg.m_databaseid = MyDatabaseId;
pgstat_send(&msg, sizeof(msg));
+
+ /* Reset the stat counters for Shared tables also. */
+ msg.m_databaseid = InvalidOid;
+ pgstat_send(&msg, sizeof(msg));

I will look into this part again. If pgstat_send forks a new process, then
I think, it will be better if we can reset stats in
pgstat_recv_resetcounter for shared tables also because shared tables are
not much in counting so it will be good if we reset in one function only. I
will debug this part more and will see.

--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2021-08-10 17:18:23 Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Previous Message Sadhuprasad Patro 2021-08-10 16:23:22 Re: Support reset of Shared objects statistics in "pg_stat_reset" function