Re: track generic and custom plans in pg_stat_statements - Mailing list pgsql-hackers

From Sami Imseih
Subject Re: track generic and custom plans in pg_stat_statements
Date
Msg-id CAA5RZ0tdc4HeHdFRvVPWBdXP1JwCSjPPgiQyFk8fwhv=4fKmjw@mail.gmail.com
Whole thread Raw
In response to Re: track generic and custom plans in pg_stat_statements  (Andrei Lepikhov <lepihov@gmail.com>)
List pgsql-hackers
> >> this for better tracking. By adding a CachedPlanSource::cplan link, we
> >> can establish a connection to the entire PlanCache entry instead of only
> >> CachedPlan within a queryDesc and, consequently, make it accessible from
> >> the executor. This would give an access to statistics on costs and the
> >> number of replannings.
> >
> > This maybe out of scope for this patch, but can you elaborate on what you mean
> > by "CachedPlanSource::cplan link" ?
> You need to introduce a 'status' field, right? - to allow someone to
> identify the plan's type, which was previously somewhat complicated.
> However, it may be implemented in a slightly different way, by adding
> CachedPlanSource::cplan (meaning 'Current Plan') and a trivial
> convention: 'cplan' references the gplan field or it refers a custom
> plan. Instead of the CachedPlan, we may provide the executor with a link
> to more stable and informative CachedPlanSource entry. That's the main idea.
> As I see it, CachedPlan doesn't make sense without plancache and a
> CachedPlanSource entry. So, it is at least a valid solution.
>
> With that link, you can access various statistics: num_custom_plans,
> num_generic_plans, total_custom_cost, and generic_cost. It would also be
> possible to clear the *_cost fields and allow Postgres to make a new
> attempt at choosing the plan type - who knows, maybe the previous
> decision is already outdated?
>
> My point is that we can address one of the common issues with generic
> plans in a more extensible way, enabling modules to access the
> CachedPlanSource data at the time they have access to the execution
> instrumentation.
>
> It seems impractical to me to invent one more patch: since you've
> already modified the CreateQueryDesc interface and introduced a plan
> type identification logic, why do it twice?

Thanks for the clarification. I see what you're getting at now.

You're suggesting adding CachedPlanSource to QueryDesc instead of
CachedPlan. This would allow extensions to access the statistics and cost
information from the CachedPlanSource, which would help tools like
pg_stat_statements track planning data, as we are trying to do with this
patch. It could also support other use cases, such as allowing extensions to
modify the costs in order to force a generic or custom plan. I had not
considered that second use case, but if there is a good case for it, I am not
opposed.

Adding CachedPlanSource to QueryDesc seems doable. However, Michael
previously objected to adding CachedPlan to QueryDesc. Is there any
similar hesitation about including CachedPlanSource?

The best way to support the functionality needed by pg_stat_statements is
to expose either CachedPlan or CachedPlanSource via QueryDesc. I
support using CachedPlanSource, since it also enables the additional use
cases that Andrei has mentioned.

Andrei, do we actually need access to CachedPlanSource::cplan? For
tracking the plan cache mode in pg_stat_statements, it be sufficient
to add a new boolean field such as is_last_plan_generic to
CachedPlanSource. Do you have another use case you have in mind
that would require a cplan field that references either the generic or
custom plan?

--
Sami



pgsql-hackers by date:

Previous
From: Nazir Bilal Yavuz
Date:
Subject: Re: Windows question: when is LC_MESSAGES defined?
Next
From: Peter Geoghegan
Date:
Subject: Re: Returning nbtree posting list TIDs in DESC order during backwards scans