Re: New SQL counter statistics view (pg_stat_sql)

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: "Smith, Peter" <peters(at)fast(dot)au(dot)fujitsu(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, vinayak <Pokale_Vinayak_q3(at)lab(dot)ntt(dot)co(dot)jp>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: New SQL counter statistics view (pg_stat_sql)
Date: 2020-03-04 21:52:56
Message-ID: 28F46159-3764-421B-904B-004DEA339310@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Mar 3, 2020, at 6:50 PM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
>
> On 2019-Nov-13, Smith, Peter wrote:
>
>> From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com> Sent: Monday, 4 November 2019 1:43 PM
>>
>>> No comment on the patch but I noticed that the documentation changes don't build. Please make sure you can "make docs" successfully, having installed the documentation tools[1].
>>
>> Thanks for the feedback. An updated patch which fixes the docs issue is attached.
>
> So, who is updating this patch for the new cmdtaglist.h stuff?

I have a much altered (competing?) patch inspired by pg_stat_sql. I am working to integrate the changes you made to my commandtag patch into my commandstats patch before posting it.

It might be good if somebody else tackled the pg_stat_sql patch in case my version is rejected.

I don't want to highjack this thread to talk in detail about the other patch, so I'll just give an overview that might be useful for anybody thinking about putting effort into committing this patch first.

The motivation for the commandtag patch that you've committed (thanks again!) was to make the commandstats patch possible. I didn't like the way pg_stat_sql stored the tags as strings, but I also still don't like the way it needs to take a lock every time a backend increments the counter for a command. That is bad, I think, because the lock overhead for an otherwise fast command (like moving a cursor) may be high enough to discourage users from turning this feature on in production.

I solve that by trading off speed and memory. For most commandtags, there is a single uint64 counter, and incrementing the counter requires taking a lock. This is very similar to how the other patch works. For the few commandtags that we deem worthy of the special treatment, there is an additional uint32 counter *per backend* reserved in shared memory. Backends update this counter using atomics, but do not require locks. When the counter is in danger of overflow, locks are taken to roll the count into the uint64 counters and zero out the backend-specific uint32 counter. I have roughly 20 command tags receiving this special treatment, but the design of the patch doesn't make it too hard to change the exact list of tags worthy of it, so I don't plan to stress too much about the exact set of tags. I'm sure you'll all tell me which ones you think do or do not deserve the treatment.

If it isn't obvious, the logic of having this treatment for only a subset of tags is that 192 tags * 4 bytes per counter * MaxBackends gets expensive for folks running a high number of backends. Taking that from 192 tags down to one or two dozen seems worthwhile.

I expect to post on a separate thread before the day is over.


Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2020-03-04 21:53:38 Re: error context for vacuum to include block number
Previous Message Justin Pryzby 2020-03-04 21:51:59 Re: error context for vacuum to include block number