Re: New SQL counter statistics view (pg_stat_sql)

On 2016/10/12 12:21, Haribabu Kommi wrote:
>
>
> On Thu, Sep 29, 2016 at 3:45 PM, Haribabu Kommi
> <kommi(dot)haribabu(at)gmail(dot)com <mailto:kommi(dot)haribabu(at)gmail(dot)com>> wrote:
>
>
>
> On Thu, Sep 22, 2016 at 3:05 AM, Alvaro Herrera
> <alvherre(at)2ndquadrant(dot)com <mailto:alvherre(at)2ndquadrant(dot)com>> wrote:
>
> Peter Eisentraut wrote:
>
> > How about having the tag not be a column name but a row
> entry. So you'd
> > do something like
> >
> > SELECT * FROM pg_stat_sql WHERE tag = 'ALTER VIEW';
> >
> > That way, we don't have to keep updating (and re-debating)
> this when new
> > command types or subtypes are added. And queries written
> for future
> > versions will not fail when run against old servers.
>
> Yeah, good idea.
>
>
> Yes, Having it as a row entry is good.
>
> Let's also discuss the interface from the stats collector.
> Currently we
> have some 20 new SQL functions, all alike, each loading the
> whole data
> and returning a single counter, and then the view invokes each
> function
> separately. That doesn't seem great to me. How about having
> a single C
> function that returns the whole thing as a SRF instead, and
> the view is
> just a single function invocation -- something like pg_lock_status
> filling pg_locks in one go.
>
> Another consideration is that the present patch lumps together
> all ALTER
> cases in a single counter. This isn't great, but at the same
> time we
> don't want to bloat the stat files by having hundreds of
> counters per
> database, do we?
>
>
> Currently, The SQL stats is a fixed size counter to track the all
> the ALTER
> cases as single counter. So while sending the stats from the
> backend to
> stats collector at the end of the transaction, the cost is same,
> because of
> it's fixed size. This approach adds overhead to send and read the
> stats
> is minimal.
>
> With the following approach, I feel it is possible to support the
> counter at
> command tag level.
>
> Add a Global and local Hash to keep track of the counters by using the
> command tag as the key, this hash table increases dynamically whenever
> a new type of SQL command gets executed. The Local Hash data is passed
> to stats collector whenever the transaction gets committed.
>
> The problem I am thinking is that, Sending data from Hash and
> populating
> the Hash from stats file for all the command tags adds some overhead.
>
>
>
> I tried changing the pg_stat_sql into row mode and ran the regress
> suite to
> add different type of SQL commands to the view and ran the pgbench test
> on my laptop to find out any performance impact with this patch.
>
> HEAD PATCH
> pgbench - select 828 816
>
> Here I attached the pg_stat_sql patch to keep track of all SQL commands
> based on the commandTag and their counts. I attached the result of this
> view that is run on the database after "make installcheck" just for
> reference.
Some comments:
I think we can use pgstat_* instead of pgstat* for code consistency.

+static HTAB *pgStatBackendSql = NULL;
How about *pgstat_backend_sql

+HTAB *pgStatSql = NULL;
How about *pgstat_sql

+static void pgstat_recv_sqlstmt(PgStat_MsgSqlstmt * msg, int len);
How about PgStat_MsgSqlstmt *msg instead of PgStat_MsgSqlstmt * msg

+typedef struct PgStatSqlstmtEntry
How about PgStat_SqlstmtEntry

+typedef struct PgStatMsgSqlstmt
How about PgStat_MsgSqlstmt

I have observed below behavior.
I have SET track_sql to ON and then execute the SELECT command and it
return 0 rows.
It start counting from the second command.
postgres=# SET track_sql TO ON;
SET
postgres=# SELECT * FROM pg_stat_sql where tag='SELECT';
tag | count
-----+-------
(0 rows)

postgres=# SELECT * FROM pg_stat_sql where tag='SELECT';
tag | count
--------+-------
SELECT | 1
(1 row)
Is this a correct behavior?

Regards,
Vinayak Pokale
NTT Open Source Software Center