Re: Is it useful to record whether plans are generic or custom?

torikoshia <torikoshia@oss.nttdata.com>

From: torikoshia <torikoshia@oss.nttdata.com>
To: Michael Paquier <michael@paquier.xyz>, legrand_legrand@hotmail.com, Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: pgsql-hackers@postgresql.org, ikedamsh@oss.nttdata.com, atorik@gmail.com, tatsuro.yamada.tf@nttcom.co.jp
Date: 2020-09-28T13:14:01Z
Lists: pgsql-hackers

Attachments

On 2020-09-17 13:46, Michael Paquier wrote:
> On Fri, Jul 31, 2020 at 06:47:48PM +0900, torikoshia wrote:
>> Oops, sorry about that.
>> I just fixed it there for now.
> 
> The regression tests of the patch look unstable, and the CF bot is
> reporting a failure here:
> https://travis-ci.org/github/postgresql-cfbot/postgresql/builds/727833416
> --
> Michael


Thank you for letting me know!


I'd like to reach a basic agreement on how we expose the
generic/custom plan information in pgss first.

Given the discussion so far, adding a new attribute to pgss key
is not appropriate since it can easily increase the number of
entries in pgss.

OTOH, just exposing the number of times generic/custom plan was
chosen seems not enough to know whether performance is degraded.

I'm now thinking about exposing not only the number of times
generic/custom plan was chosen but also some performance
metrics like 'total_time' for both generic and custom plans.

Attached a poc patch which exposes total, min, max, mean and
stddev time for both generic and custom plans.


   =# SELECT * FROM =# SELECT * FROM pg_stat_statements;
   -[ RECORD 1 
]-------+---------------------------------------------------------
   userid              | 10
   dbid                | 12878
   queryid             | 4617094108938234366
   query               | PREPARE pr1 AS SELECT * FROM pg_class WHERE 
relname = $1
   plans               | 0
   total_plan_time     | 0
   min_plan_time       | 0
   max_plan_time       | 0
   mean_plan_time      | 0
   stddev_plan_time    | 0
   calls               | 6
   total_exec_time     | 0.46600699999999995
   min_exec_time       | 0.029376000000000003
   max_exec_time       | 0.237413
   mean_exec_time      | 0.07766783333333334
   stddev_exec_time    | 0.07254973134206326
   generic_calls       | 1
   total_generic_time  | 0.045334000000000006
   min_generic_time    | 0.045334000000000006
   max_generic_time    | 0.045334000000000006
   mean_generic_time   | 0.045334000000000006
   stddev_generic_time | 0
   custom_calls        | 5
   total_custom_time   | 0.42067299999999996
   min_custom_time     | 0.029376000000000003
   max_custom_time     | 0.237413
   mean_custom_time    | 0.0841346
   stddev_custom_time  | 0.07787966226583164
   ...

In this patch, exposing new columns is mandatory, but I think
it's better to make it optional by adding a GUC something
like 'pgss.track_general_custom_plans.

I also feel it makes the number of columns too many.
Just adding the total time may be sufficient.


Any thoughts?


Regards,

--
Atsushi Torikoshi

Commits

  1. Add generic_plans and custom_plans fields into pg_prepared_statements.