From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: plan cache overhead on plpgsql expression |
Date: | 2020-03-26 19:05:20 |
Message-ID: | 14781.1585249520@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2020-03-26 14:37:59 -0400, Tom Lane wrote:
>> + * This function, together with CachedPlanIsSimplyValid, provides a fast path
>> + * for revalidating "simple" generic plans. The core requirement to be simple
>> + * is that the plan must not require taking any locks, which translates to
>> + * not touching any tables; this happens to match up well with an important
>> + * use-case in PL/pgSQL.
> Hm - is there currently sufficient guarantee that we absorb sinval
> messages? Would still matter for types, functions, etc?
There are potentially issues of that sort throughout the backend, not
just here, since we don't have any locking on types or functions.
I don't think it's this patch's job to address that. In practice
I think we've thought about it and concluded that the cost/benefit
of introducing such locks just isn't promising:
* Generally speaking you can't do anything very interesting to a type
anyway, at least not with supported DDL. The worst-case situation that
could materialize AFAIK is possibly evaluating slightly-stale constraints
for a domain. (The typcache does have sinval invalidation for those
constraints, but I don't recall offhand how much we guarantee about
how quickly we'll notice updates.)
* For functions, you might execute a somewhat stale version of the
function body. The bad effects there are pretty limited since a function
is defined by just one catalog row, unlike tables, so you can't see a
self-inconsistent version of it.
The amount of lock overhead that it would take to remove those edge
cases seems slightly staggering, so I doubt we'd ever do it.
> While it'd do a small bit unnecessary work, I do wonder if it'd be
> better to use this code in ResourceOwnereReleaseInternal().
When and if we refactor to expose this sort of thing more generally,
it might be worth doing it like that. I don't think it helps much
right now.
> Perhaps add a reference to the new (appreciated, btw) DO comment above?
Can do.
Again, thanks for reviewing!
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Alexey Kondratov | 2020-03-26 19:22:08 | Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly |
Previous Message | Andres Freund | 2020-03-26 18:50:56 | Re: Berserk Autovacuum (let's save next Mandrill) |