From: | torikoshia <torikoshia(at)oss(dot)nttdata(dot)com> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
Cc: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, ikedamsh(at)oss(dot)nttdata(dot)com, atorik(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org, legrand_legrand(at)hotmail(dot)com, tatsuro(dot)yamada(dot)tf(at)nttcom(dot)co(dot)jp |
Subject: | Re: Is it useful to record whether plans are generic or custom? |
Date: | 2020-07-10 01:49:11 |
Message-ID: | 6741cff72d0e61c8e64ca45687fe2038@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2020-07-08 16:41, Fujii Masao wrote:
> On 2020/07/08 10:14, torikoshia wrote:
>> On 2020-07-06 22:16, Fujii Masao wrote:
>>> On 2020/06/11 14:59, torikoshia wrote:
>>>> On 2020-06-10 18:00, Kyotaro Horiguchi wrote:
>>>>
>>>>>
>>>>> + TupleDescInitEntry(tupdesc, (AttrNumber) 8, "last_plan",
>>>>>
>>>>> This could be a problem if we showed the last plan in this view. I
>>>>> think "last_plan_type" would be better.
>>>>>
>>>>> + if (prep_stmt->plansource->last_plan_type ==
>>>>> PLAN_CACHE_TYPE_CUSTOM)
>>>>> + values[7] = CStringGetTextDatum("custom");
>>>>> + else if (prep_stmt->plansource->last_plan_type ==
>>>>> PLAN_CACHE_TYPE_GENERIC)
>>>>> + values[7] = CStringGetTextDatum("generic");
>>>>> + else
>>>>> + nulls[7] = true;
>>>>>
>>>>> Using swith-case prevents future additional type (if any) from
>>>>> being
>>>>> unhandled. I think we are recommending that as a convension.
>>>>
>>>> Thanks for your reviewing!
>>>>
>>>> I've attached a patch that reflects your comments.
>>>
>>> Thanks for the patch! Here are the comments.
>>
>> Thanks for your review!
>>
>>> + Number of times generic plan was choosen
>>> + Number of times custom plan was choosen
>>>
>>> Typo: "choosen" should be "chosen"?
>>
>> Thanks, fixed them.
>>
>>> + <entry role="catalog_table_entry"><para
>>> role="column_definition">
>>> + <structfield>last_plan_type</structfield> <type>text</type>
>>> + </para>
>>> + <para>
>>> + Tells the last plan type was generic or custom. If the
>>> prepared
>>> + statement has not executed yet, this field is null
>>> + </para></entry>
>>>
>>> Could you tell me how this information is expected to be used?
>>> I think that generic_plans and custom_plans are useful when
>>> investigating
>>> the cause of performance drop by cached plan mode. But I failed to
>>> get
>>> how much useful last_plan_type is.
>>
>> This may be an exceptional case, but I once had a case needed
>> to ensure whether generic or custom plan was chosen for specific
>> queries in a development environment.
>
> In your case, probably you had to ensure that the last multiple (or
> every)
> executions chose generic or custom plan? If yes, I'm afraid that
> displaying
> only the last plan mode is not enough for your case. No?
> So it seems better to check generic_plans or custom_plans columns in
> the
> view rather than last_plan_type even in your case. Thought?
Yeah, I now feel last_plan is not so necessary and only the numbers of
generic/custom plan is enough.
If there are no objections, I'm going to remove this column and related
codes.
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2020-07-10 01:49:13 | Re: Resetting spilled txn statistics in pg_stat_replication |
Previous Message | Michael Paquier | 2020-07-10 01:45:57 | Re: Physical replication slot advance is not persistent |