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

From: Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>
To: Atsushi Torikoshi <atorik(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, legrand legrand <legrand_legrand(at)hotmail(dot)com>, Tatsuro Yamada <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-06-08 11:45:15
Message-ID: ee7e5455d039b00849c62e1172d8035c@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2020-06-04 17:04, Atsushi Torikoshi wrote:
> On Mon, May 25, 2020 at 10:54 AM Atsushi Torikoshi <atorik(at)gmail(dot)com>
> wrote:
>
>> On Thu, May 21, 2020 at 5:10 PM Kyotaro Horiguchi
>> <horikyota(dot)ntt(at)gmail(dot)com> wrote:
>>
>>> Cost numbers would look better if it is cooked a bit. Is it worth
>>> being in core?
>>
>> I didn't come up with ideas about how to use them.
>> IMHO they might not so necessary..
>
>>> Perhaps plan_generation is not needed there.
>>
>> +1.
>> Now 'calls' is sufficient and 'plan_generation' may confuse users.
>>
>> BTW, considering 'calls' in pg_stat_statements is the number of
>> times
>> statements were EXECUTED and 'plans' is the number of times
>> statements were PLANNED, how about substituting 'calls' for
>> 'plans'?
>
> I've modified the above points and also exposed the numbers of each
> generic plans and custom plans.
>
> I'm now a little bit worried about the below change which removed
> the overflow checking for num_custom_plans, which was introduced
> in 0001-Expose-counters-of-plancache-to-pg_prepared_statement.patch,
> but I've left it because the maximum of int64 seems enough large
> for counters.
> Also referencing other counters in pg_stat_user_tables, they don't
> seem to care about it.
>
> ```
> - /* Accumulate total costs of custom plans, but 'ware
> overflow */
> - if (plansource->num_custom_plans < INT_MAX)
> - {
> - plansource->total_custom_cost +=
> cached_plan_cost(plan, true);
> - plansource->num_custom_plans++;
> - }
>
> ```
>
> Regards,
>
> Atsushi Torikoshi
>
>>

As a user, I think this feature is useful for performance analysis.
I hope that this feature is merged.

BTW, I found that the dependency between function's comments and
the modified code is broken at latest patch. Before this is
committed, please fix it.

```
diff --git a/src/backend/commands/prepare.c
b/src/backend/commands/prepare.c
index 990782e77f..b63d3214df 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -694,7 +694,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt,
IntoClause *into, ExplainState *es,

/*
* This set returning function reads all the prepared statements and
- * returns a set of (name, statement, prepare_time, param_types,
from_sql).
+ * returns a set of (name, statement, prepare_time, param_types,
from_sql,
+ * generic_plans, custom_plans, last_plan).
*/
Datum
pg_prepared_statement(PG_FUNCTION_ARGS)
```

Regards,

--
Masahiro Ikeda

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2020-06-08 12:44:31 TAP tests and symlinks on Windows
Previous Message Asif Rehman 2020-06-08 11:39:18 Re: proposal - plpgsql - FOR over unbound cursor