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

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 reason for 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 be zero)

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