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