Re: Planning counters in pg_stat_statements (using pgss_store)

Hello

On 2019/09/06 23:19, Tomas Vondra wrote:
> On Wed, Sep 04, 2019 at 07:19:47PM +0300, Sergei Kornilov wrote:
>>
>> ...
>>
>> Results:
>>
>>         test         |   mode   | average_tps | degradation_perc
>> ----------------------+----------+-------------+------------------
>> head_no_pgss         | extended |       13816 |            1.000
>> patch_not_loaded     | extended |       13755 |            0.996
>> head_track_none      | extended |       13607 |            0.985
>> patch_track_none     | extended |       13560 |            0.981
>> head_track_top       | extended |       13277 |            0.961
>> patch_track_top      | extended |       13189 |            0.955
>> patch_track_planning | extended |       12983 |            0.940
>> head_no_pgss         | prepared |       29101 |            1.000
>> head_track_none      | prepared |       28510 |            0.980
>> patch_track_none     | prepared |       28481 |            0.979
>> patch_not_loaded     | prepared |       28382 |            0.975
>> patch_track_planning | prepared |       28046 |            0.964
>> head_track_top       | prepared |       28035 |            0.963
>> patch_track_top      | prepared |       27973 |            0.961
>> head_no_pgss         | simple   |       16733 |            1.000
>> patch_not_loaded     | simple   |       16552 |            0.989
>> head_track_none      | simple   |       16452 |            0.983
>> patch_track_none     | simple   |       16365 |            0.978
>> head_track_top       | simple   |       15867 |            0.948
>> patch_track_top      | simple   |       15820 |            0.945
>> patch_track_planning | simple   |       15739 |            0.941
>>
>> So I found slight slowdown with track_planning = off compared to HEAD. 
>> Possibly just at the level of measurement error. I think this is ok.
>> track_planning = on also has no dramatic impact. In my opinion 
>> proposed design with pgss_store call is acceptable.
>>
> 
> FWIW I've done some benchmarking on this too, with a single pgbench client
> running select-only test on a tiny database, in different modes (simple,
> extended, prepared). I've done that on two systems with different CPUs
> (spreadsheet with results attached).

Refering to Sergei's results, if a user, currently using pgss with 
tracking execute time, uses the new feature, a user will see 0~2.2% 
performance regression as below.

 >> head_track_top       | extended |       13277 |            0.961
 >> patch_track_planning | extended |       12983 |            0.940
 >> patch_track_planning | prepared |       28046 |            0.964
 >> head_track_top       | prepared |       28035 |            0.963
 >> head_track_top       | simple   |       15867 |            0.948
 >> patch_track_planning | simple   |       15739 |            0.941

If a user will not turn on the track_planning, a user will see 0.2-0.6% 
performance regression as below.

 >> head_track_top       | extended |       13277 |            0.961
 >> patch_track_top      | extended |       13189 |            0.955
 >> head_track_top       | prepared |       28035 |            0.963
 >> patch_track_top      | prepared |       27973 |            0.961
 >> head_track_top       | simple   |       15867 |            0.948
 >> patch_track_top      | simple   |       15820 |            0.945

> 
> I don't see any performance regression - there are some small variations
> in both directions (say, ~1%) but that's well within the noise. So I think
> the patch is fine in this regard.

+1


I also saw the codes and have one comment.

[0002 patch]
In pgss_planner_hook:

+        /* calc differences of buffer counters. */
+        bufusage = compute_buffer_counters(bufusage_start, pgBufferUsage);
+
+        /*
+         * we only store planning duration, query text has been initialized
+         * during previous pgss_post_parse_analyze as it not available inside
+         * pgss_planner_hook.
+         */
+        pgss_store(query_text,

Do we need to calculate bufusage in here?
We only store planning duration in the following pgss_store.

--
Yoshikazu Imai