On Tue, 10 Aug 2021 at 22:32, Mahendra Singh Thalor <mahi6...@gmail.com> wrote: > > On Tue, 10 Aug 2021 at 21:53, Sadhuprasad Patro <b.sa...@gmail.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. >
I checked this and found that we already have one process "stats collector" and in v02 patch, we are sending requests to collect stats two times. I don't know how costly one call is but I feel that we can avoid the 2nd call by keeping handling of the shared tables into pgstat_recv_resetcounter. Thoughts? -- Thanks and Regards Mahendra Singh Thalor EnterpriseDB: http://www.enterprisedb.com
