Re: Support reset of Shared objects statistics in "pg_stat_reset" function - Mailing list pgsql-hackers

Hi,

> > 3)
> >  pgstat_recv_resetsinglecounter(PgStat_MsgResetsinglecounter *msg, int len)
> >  {
> >      PgStat_StatDBEntry *dbentry;
> > +    bool        found;
> >
> >      dbentry = pgstat_get_db_entry(msg->m_databaseid, false);
> >
> > @@ -5168,13 +5192,41 @@
> > pgstat_recv_resetsinglecounter(PgStat_MsgResetsinglecounter *msg, int
> > len)
> >      /* Set the reset timestamp for the whole database */
> >      dbentry->stat_reset_timestamp = GetCurrentTimestamp();
> >
> > -    /* Remove object if it exists, ignore it if not */
> > +    /* Remove object if it exists, if not then may be it's a shared table */
> >      if (msg->m_resettype == RESET_TABLE)
> > +    {
> >          (void) hash_search(dbentry->tables, (void *) &(msg->m_objectid),
> > -                           HASH_REMOVE, NULL);
> > +                           HASH_REMOVE, &found);
> > +        if (!found)
> > +        {
> > +            /* If we didn't find it, maybe it's a shared table */
> > +            dbentry = pgstat_get_db_entry(InvalidOid, false);
> > +            if (dbentry)
> > +            {
> > +                /* Set the reset timestamp for the whole database */
> > +                dbentry->stat_reset_timestamp = GetCurrentTimestamp();
> > +                (void) hash_search(dbentry->tables, (void *)
> > &(msg->m_objectid),
> > +                                   HASH_REMOVE, NULL);
> > +            }
> > +        }
> > +    }
> >      else if (msg->m_resettype == RESET_FUNCTION)
> > +    {
> >          (void) hash_search(dbentry->functions, (void *) &(msg->m_objectid),
> > -                           HASH_REMOVE, NULL);
> > +                           HASH_REMOVE, &found);
> > +        if (!found)
> > +        {
> > +            /* If we didn't find it, maybe it's a shared table */
> > +            dbentry = pgstat_get_db_entry(InvalidOid, false);
> > +            if (dbentry)
> > +            {
> > +                /* Set the reset timestamp for the whole database */
> > +                dbentry->stat_reset_timestamp = GetCurrentTimestamp();
> > +                (void) hash_search(dbentry->functions, (void *)
> > &(msg->m_objectid),
> > +                                   HASH_REMOVE, NULL);
> > +            }
> > +        }
> > +    }
> >  }
> >
> > Above code can be replaced with:
> > @@ -5160,7 +5162,10 @@
> > pgstat_recv_resetsinglecounter(PgStat_MsgResetsinglecounter *msg, int
> > len)
> >  {
> >         PgStat_StatDBEntry *dbentry;
> >
> > -       dbentry = pgstat_get_db_entry(msg->m_databaseid, false);
> > +       if (IsSharedRelation(msg->m_objectid))
> > +               dbentry = pgstat_get_db_entry(InvalidOid, false);
> > +       else
> > +               dbentry = pgstat_get_db_entry(msg->m_databaseid, false);
> >
> >         if (!dbentry)
> >                 return;
> >

Will Check the function "IsSharedRelation '' usage further and will
use it to fix the patch.
But I have not seen this function used every time as needed. No where
in the stat collector code.


> > 5) pg_stat_reset_single_table_counters is not resetting all the
> > columns for pg_database.
> > Ex:
> > postgres=# SELECT * FROM pg_statio_all_tables  where relid =
> > 'pg_database'::regclass::oid;
> >  relid | schemaname |   relname   | heap_blks_read | heap_blks_hit |
> > idx_blks_read | idx_blks_hit | toast_blks_read | toast_blks_hit |
> > tidx_blks_read | tidx_blks_hit
> >
-------+------------+-------------+----------------+---------------+---------------+--------------+-----------------+----------------+----------------+---------------
> >   1262 | pg_catalog | pg_database |              1 |             2 |
> >           2 |            0 |               0 |              0 |
> >       0 |             0
> > (1 row)
> >
> > postgres=# select
> > pg_stat_reset_single_table_counters('pg_database'::regclass::oid);
> >  pg_stat_reset_single_table_counters
> > -------------------------------------
> >
> > (1 row)
> >
> > postgres=# SELECT * FROM pg_statio_all_tables  where relid =
> > 'pg_database'::regclass::oid;
> >  relid | schemaname |   relname   | heap_blks_read | heap_blks_hit |
> > idx_blks_read | idx_blks_hit | toast_blks_read | toast_blks_hit |
> > tidx_blks_read | tidx_blks_hit
> >
-------+------------+-------------+----------------+---------------+---------------+--------------+-----------------+----------------+----------------+---------------
> >   1262 | pg_catalog | pg_database |              0 |             0 |
> >           2 |            0 |               0 |              0 |
> >       0 |             0
> > (1 row)
> >

As we have given input as ObjectID of pg_database, it resets only the
counters of heap_blk_read & heap_blk_hit.
To reset other counters like index & toast table related, we need to
provide respective objectIDs and call the function again. Then it
resets everything.
This behavior is kept same as user tables local to the database.



> 2) I think, pg_stat_reset_single_table_counters is not working properly for shared tables so we should find out the
reasonfor this.
 
>
> Ex: (I tested for pg_database)
> postgres=# SELECT * FROM pg_statio_all_tables  where relid = 'pg_database'::regclass::oid;
>  relid | schemaname |   relname   | heap_blks_read | heap_blks_hit | idx_blks_read | idx_blks_hit | toast_blks_read |
toast_blks_hit| tidx_blks_read | tidx_blks_hit
 
>
-------+------------+-------------+----------------+---------------+---------------+--------------+-----------------+----------------+----------------+---------------
>   1262 | pg_catalog | pg_database |              1 |             2 |             2 |            0 |               0 |
            0 |              0 |             0
 
> (1 row)
> postgres=#
> postgres=# select pg_stat_reset_single_table_counters('pg_database'::regclass::oid);
>  pg_stat_reset_single_table_counters
> -------------------------------------
>
> (1 row)
> postgres=# SELECT * FROM pg_statio_all_tables  where relid = 'pg_database'::regclass::oid;
>  relid | schemaname |   relname   | heap_blks_read | heap_blks_hit | idx_blks_read | idx_blks_hit | toast_blks_read |
toast_blks_hit| tidx_blks_read | tidx_blks_hit
 
>
-------+------------+-------------+----------------+---------------+---------------+--------------+-----------------+----------------+----------------+---------------
>   1262 | pg_catalog | pg_database |              0 |             0 |             2 |            0 |               0 |
            0 |              0 |             0
 
> (1 row)

This function "pg_stat_reset_single_table_counters" works as per the
given table ID. To reset the index & toast table related counters, we
need to provide the respective ObjectIDs. (Refer example given for
pg_attribute).
So this works the same as a shared table and user tables local to the
current database.

> 3) I am attaching a .sql file. We can add similar types of test cases for shared tables.
> Ex: first reset stats for all shared tables using pg_stat_reset and then check stats for all shared tables(all should
bezero)
 

I did not find any existing test case for "pg_stat_reset", so I did
not add a new testcase for this issue..
I can add if needed.

-- 
Thanks & Regards
SadhuPrasad
EnterpriseDB: http://www.enterprisedb.com