Re: [HACKERS] More efficient truncation of pg_stat_activity query strings

On Thu, Sep 14, 2017 at 11:33 AM, Andres Freund <andres@anarazel.de> wrote:
> On 2017-09-12 00:19:48 -0700, Andres Freund wrote:
>> Hi,
>>
>> I've recently seen a benchmark in which pg_mbcliplen() showed up
>> prominently. Which it will basically in any benchmark with longer query
>> strings, but fast queries. That's not that uncommon.
>>
>> I wonder if we could avoid the cost of pg_mbcliplen() from within
>> pgstat_report_activity(), by moving some of the cost to the read
>> side. pgstat values are obviously read far less frequently in nearly all
>> cases that are performance relevant.
>>
>> Therefore I wonder if we couldn't just store a querystring that's
>> essentially just a memcpy()ed prefix, and do a pg_mbcliplen() on the
>> read side.  I think that should work because all *server side* encodings
>> store character lengths in the *first* byte of a multibyte character
>> (at least one clientside encoding, gb18030, doesn't behave that way).
>>
>> That'd necessitate an added memory copy in pg_stat_get_activity(), but
>> that seems fairly harmless.
>>
>> Faults in my thinking?
>
> Here's a patch that implements that idea.  Seems to work well.  I'm a
> bit loathe to add proper regression tests for this, seems awfully
> dependent on specific track_activity_query_size settings.  I did confirm
> using gdb that I see incomplete characters before
> pgstat_clip_activity(), but not after.
>
> I've renamed st_activity to st_activity_raw to increase the likelihood
> that potential external users of st_activity notice and adapt. Increases
> the noise, but imo to a very bareable amount. Don't feel strongly
> though.
>
Hello,

The patch looks good to me. I've done some regression testing with a
custom script on my local system. The script contains the following
statement:
SELECT 'aaa..<repeated 600 times>' as col;

Test 1
-----------------------------------
duration: 300 seconds
clients/threads: 1

On HEAD TPS: 13181
+    9.30%     0.27%  postgres  postgres             [.] pgstat_report_activity
+    8.88%     0.02%  postgres  postgres             [.] pg_mbcliplen
+    7.76%     4.77%  postgres  postgres             [.] pg_encoding_mbcliplen
+    4.06%     4.06%  postgres  postgres             [.] pg_utf_mblen

With the patch TPS:13628 (+3.39%)
+    0.36%     0.21%  postgres  postgres  [.] pgstat_report_activity

Test 2
-----------------------------------
duration: 300 seconds
clients/threads: 8

On HEAD TPS: 53084
+   12.17%     0.20%  postgres  postgres             [.]
pgstat_report_activity
+   11.83%     0.02%  postgres  postgres             [.] pg_mbcliplen
+   11.19%     8.03%  postgres  postgres             [.] pg_encoding_mbcliplen
+    3.74%     3.73%  postgres  postgres             [.] pg_utf_mblen

With the patch TPS: 63949 (+20.4%)
+    0.40%     0.25%  postgres  postgres  [.] pgstat_report_activity

This shows the significance of this patch in terms of performance
improvement of pgstat_report_activity. Is there any other tests I
should do for the same?


-- 
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers