On 2016/10/12 12:21, Haribabu Kommi wrote:


On Thu, Sep 29, 2016 at 3:45 PM, Haribabu Kommi <kommi.harib...@gmail.com <mailto:kommi.harib...@gmail.com>> wrote:



    On Thu, Sep 22, 2016 at 3:05 AM, Alvaro Herrera
    <alvhe...@2ndquadrant.com <mailto:alvhe...@2ndquadrant.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


Reply via email to