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

On Tue, 10 Aug 2021 at 22:32, Mahendra Singh Thalor <mahi6run(at)gmail(dot)com> wrote:
>
> 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.
>

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