> On Mar 3, 2020, at 6:50 PM, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:
>
> On 2019-Nov-13, Smith, Peter wrote:
>
>> From: Thomas Munro <thomas.mu...@gmail.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
