Обсуждение: DISCARD ALL does not force re-planning of plpgsql functions/procedures

Поиск
Список
Период
Сортировка

DISCARD ALL does not force re-planning of plpgsql functions/procedures

От
Jelte Fennema-Nio
Дата:
I got a report on the PgBouncer repo[1] that running DISCARD ALL was
not sufficient between connection handoffs to force replanning of
stored procedures. Turns out that while DISCARD AL and DISCARD PLAN
reset the plan cache they do not reset the num_custom_plans fields of
the existing PlanSources. So while the generic plan is re-planned
after DISCARD ALL, the decision on whether to choose it or not won't
be changed. See below for a minimally reproducing example:


create table test_mode (a int);
insert into test_mode select 1 from generate_series(1,1000000) union
all select 2;
create index on test_mode (a);
analyze test_mode;

create function test_mode_func(int)
returns integer as $$
declare
    v_count integer;
begin
    select into v_count count(*) from test_mode where a = $1;
    return v_count;
END;
$$ language plpgsql;

\timing on
-- trigger execution 5 times
SELECT test_mode_func(1);
SELECT test_mode_func(1);
SELECT test_mode_func(1);
SELECT test_mode_func(1);
SELECT test_mode_func(1);
DISCARD ALL;
-- slow because of bad plan, even after DISCARD ALL
SELECT test_mode_func(2);
\c
-- fast after re-connect, because of custom plan
SELECT test_mode_func(2);



[1]: https://github.com/pgbouncer/pgbouncer/issues/912#issuecomment-2131250109



Jelte Fennema-Nio <postgres@jeltef.nl> writes:
> I got a report on the PgBouncer repo[1] that running DISCARD ALL was
> not sufficient between connection handoffs to force replanning of
> stored procedures. Turns out that while DISCARD AL and DISCARD PLAN
> reset the plan cache they do not reset the num_custom_plans fields of
> the existing PlanSources. So while the generic plan is re-planned
> after DISCARD ALL, the decision on whether to choose it or not won't
> be changed.

Hm, should it be?  That's hard-won knowledge, and I'm not sure there
is a good reason to believe it's no longer applicable.

Note that any change in behavior there would affect prepared
statements in general, not only plpgsql.

            regards, tom lane



Re: DISCARD ALL does not force re-planning of plpgsql functions/procedures

От
Jelte Fennema-Nio
Дата:
On Sun, 26 May 2024 at 19:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Hm, should it be?  That's hard-won knowledge, and I'm not sure there
> is a good reason to believe it's no longer applicable.

I think for DISCARD ALL it would probably make sense to forget this knowledge . Since that is advertised as "reset the session to its initial state". DISCARD PLANS should probably forget about it though indeed. 

> Note that any change in behavior there would affect prepared
> statements in general, not only plpgsql.

DISCARD ALL already removes all prepared statements and thus their run counts, so for prepared statements there would be no difference there. 

Re: DISCARD ALL does not force re-planning of plpgsql functions/procedures

От
Pavel Stehule
Дата:


ne 26. 5. 2024 v 21:27 odesílatel Jelte Fennema-Nio <postgres@jeltef.nl> napsal:
On Sun, 26 May 2024 at 19:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Hm, should it be?  That's hard-won knowledge, and I'm not sure there
> is a good reason to believe it's no longer applicable.

I think for DISCARD ALL it would probably make sense to forget this knowledge . Since that is advertised as "reset the session to its initial state". DISCARD PLANS should probably forget about it though indeed. 

> Note that any change in behavior there would affect prepared
> statements in general, not only plpgsql.

DISCARD ALL already removes all prepared statements and thus their run counts, so for prepared statements there would be no difference there.

+1

Pavel

Re: DISCARD ALL does not force re-planning of plpgsql functions/procedures

От
Jelte Fennema-Nio
Дата:


On Sun, May 26, 2024, 12:26 Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
DISCARD PLANS should probably forget about it though indeed. 

DISCARD PLANS should probably **not** forget about it


> Note that any change in behavior there would affect prepared
> statements in general, not only plpgsql.

DISCARD ALL already removes all prepared statements and thus their run counts, so for prepared statements there would be no difference there. 

Re: DISCARD ALL does not force re-planning of plpgsql functions/procedures

От
Jelte Fennema-Nio
Дата:
On Sun, 26 May 2024 at 19:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Hm, should it be?  That's hard-won knowledge, and I'm not sure there
> is a good reason to believe it's no longer applicable.

Okay, so I looked into this a bit more and there's a similar case here
that's imho very clearly doing something incorrectly: num_custom_plans
of prepared statements is not reset when you change the search_path.
When the search_path is changed, there's no reason to assume that the
previous generic plans have any relevance to the new generic plans,
because the tables that are being accessed might be completely
different. See below for an (imho) obviously incorrect choice of using
the generic plan after changing search_path. Maybe the fix for this
issue should be that if a plan gets invalidated, then num_custom_plans
for the source of that plan should be set to zero too. So to be clear,
that means I now think that DISCARD PLANS should also reset
num_custom_plans (as opposed to what I said before).

create schema a;
create schema b;
create table a.test_mode (a int);
create table b.test_mode (a int);
insert into a.test_mode select 1 from generate_series(1,1000000) union
all select 2;
insert into b.test_mode select 2 from generate_series(1,1000000) union
all select 1;
create index on a.test_mode (a);
create index on b.test_mode (a);
analyze a.test_mode;
analyze b.test_mode;

SET search_path = a;
PREPARE test_mode_func(int) as select count(*) from test_mode where a = $1;

\timing on
-- trigger execution 5 times
EXECUTE test_mode_func(1);
EXECUTE test_mode_func(1);
EXECUTE test_mode_func(1);
EXECUTE test_mode_func(1);
EXECUTE test_mode_func(1);
-- slow because of bad plan, even after changing search_path
SET search_path = b;
EXECUTE test_mode_func(1);
\c
-- fast after re-connect, because of custom plan
SET search_path = a;
PREPARE test_mode_func(int) as select count(*) from test_mode where a = $1;
SET search_path = b;
EXECUTE test_mode_func(1);