pg_temp_%d namespace creation can invalidate all the cached plan in other backends

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

pg_temp_%d namespace creation can invalidate all the cached plan in other backends

Andy Fan
Planning is expensive and we use plancache to bypass its effect. I find the
$subject recently which is caused by we register NAMESPACEOID invalidation
message for pg_temp_%s as well as other normal namespaces.  Is it a
must?

We can demo the issue with the below case:

Sess1: 
create table t (a int);
prepare s as select * from t;
postgres=# execute s;
INFO:  There is no cached plan now
 a
---
(0 rows)

postgres=# execute s;  -- The plan is cached.
 a
---
(0 rows)


Sess2:
create temp table m (a int);

Sess1:

postgres=# execute s;  -- The cached plan is reset.
INFO:  There is no cached plan now
 a
---
(0 rows)


What I want to do now is bypass the invalidation message totally if it is a pg_temp_%d
namespace.  (RELATION_IS_OTHER_TEMP). With this change, the impact is not only
the plan cache is not reset but also all the other stuff in
SysCacheInvalidate/CallSyscacheCallbacks will not be called (for pg_temp_%d change
only).  I think pg_temp_%d is not meaningful for others, so I think the bypassing is OK.
I still have not kicked off any coding so far, I want to know if it is a correct thing to do?

--
Best Regards
Reply | Threaded
Open this post in threaded view
|

Re: pg_temp_%d namespace creation can invalidate all the cached plan in other backends

Andy Fan


On Tue, Feb 23, 2021 at 12:07 PM Andy Fan <[hidden email]> wrote:
Planning is expensive and we use plancache to bypass its effect. I find the
$subject recently which is caused by we register NAMESPACEOID invalidation
message for pg_temp_%s as well as other normal namespaces.  Is it a
must?

We can demo the issue with the below case:

Sess1: 
create table t (a int);
prepare s as select * from t;
postgres=# execute s;
INFO:  There is no cached plan now
 a
---
(0 rows)

postgres=# execute s;  -- The plan is cached.
 a
---
(0 rows)


Sess2:
create temp table m (a int);

Sess1:

postgres=# execute s;  -- The cached plan is reset.
INFO:  There is no cached plan now
 a
---
(0 rows)


What I want to do now is bypass the invalidation message totally if it is a pg_temp_%d
namespace.  (RELATION_IS_OTHER_TEMP).

Please ignore the word "RELATION_IS_OTHER_TEMP", it is pasted here by accident.. 
 
With this change, the impact is not only
the plan cache is not reset but also all the other stuff in
SysCacheInvalidate/CallSyscacheCallbacks will not be called (for pg_temp_%d change
only).  I think pg_temp_%d is not meaningful for others, so I think the bypassing is OK.
I still have not kicked off any coding so far, I want to know if it is a correct thing to do?

--
Best Regards


--
Best Regards
Reply | Threaded
Open this post in threaded view
|

Re: pg_temp_%d namespace creation can invalidate all the cached plan in other backends

Tom Lane-2
In reply to this post by Andy Fan
Andy Fan <[hidden email]> writes:
> Planning is expensive and we use plancache to bypass its effect. I find the
> $subject recently which is caused by we register NAMESPACEOID invalidation
> message for pg_temp_%s as well as other normal namespaces.  Is it a
> must?

Since we don't normally delete those namespaces once they exist,
the number of such events is negligible over the life of a database
(at least in production scenarios).  I'm having a very hard time
getting excited about spending effort here.

Also, you can't just drop the inval event, because even if you
believe it's irrelevant to other backends (a questionable
assumption), it certainly is relevant locally.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: pg_temp_%d namespace creation can invalidate all the cached plan in other backends

Andy Fan


On Tue, Feb 23, 2021 at 1:50 PM Tom Lane <[hidden email]> wrote:
Andy Fan <[hidden email]> writes:
> Planning is expensive and we use plancache to bypass its effect. I find the
> $subject recently which is caused by we register NAMESPACEOID invalidation
> message for pg_temp_%s as well as other normal namespaces.  Is it a
> must?

Since we don't normally delete those namespaces once they exist,
the number of such events is negligible over the life of a database
(at least in production scenarios). 

I do miss this part during my test.  Thanks for sharing this.  
 
I'm having a very hard time
getting excited about spending effort here.

While I admit this should happen rarely in production, I still think we
may need to fix it.  This is kind of tech debt. For example, why my
application has a spike on time xx:yy:zz (Assume it happens even
it is rare).  I think there is a very limited DBA who can find out this
reason easily.  Even he can find out it, he is still hard to make others
to understand and be convinced.  So why shouldn't we just avoid it 
if the effort is not huge?  

(I do find this in my production case, where the case starts
from this invalidation message, and crashes at ResetPlanCache.
I'm using a modified version, so the crash probably not the community
version's fault and we will fix it separately. )

Also, you can't just drop the inval event, because even if you
believe it's irrelevant to other backends (a questionable
assumption), it certainly is relevant locally.

Thanks for this hint!  Can just finding a place to run
SysCacheInvalidate/CallSyscacheCallbacks locally fix this issue?

--
Best Regards