Thread: Plan invalidation
I noticed that the plan invalidation is not immediately effective. Not sure whether it's worth fixing or has any other side-effects, but thought would just post it. I was testing the following scenario: session1 session2 CREATE TABLE test (int a, int b); INSERT INTO TABLE SET enable_seqscan = off BEGIN PREPARE myplan AS SELECT * FROM TEST WHERE a = 100; EXPLAIN EXECUTE myplan; (seq scan) CREATE INDEX -----> EXPLAIN EXECUTE myplan (seq scan) EXPLAINEXECUTE myplan (index scan) The second "EXPLAIN" in session 2 uses seq scan because the plan is not invalidated and replanned properly. Ideally it should have used the index scan. I traced it a bit and it seems that the invalidation messages are not accepted in session 2 because the locks are already held on the relation. At the end of the command, session 2 calls CommandCounterIncrement() and gets the invalidation messages. Hence the next EXPLAIN revalidates the plan properly. May be this is not such an important issue. But I was wondering if there are other places in the code where we might miss or receive invalidation messages with a delay, mostly because of *lack* of lock conflict ? Thanks, Pavan -- EnterpriseDB http://www.enterprisedb.com
"Pavan Deolasee" <pavan.deolasee@enterprisedb.com> writes: > I traced it a bit and it seems that the invalidation messages > are not accepted in session 2 because the locks are already held > on the relation. Right, because of this coding in LockRelationOid(): /* * Now that we have the lock, check for invalidation messages, so that we * will update or flush any stale relcacheentry before we try to use it. * We can skip this in the not-uncommon case that we already had the same * typeof lock being requested, since then no one else could have * modified the relcache entry in an undesirable way. (Inthe case where * our own xact modifies the rel, the relcache update happens via * CommandCounterIncrement, not here.) */ if (res != LOCKACQUIRE_ALREADY_HELD) AcceptInvalidationMessages(); We could remove the optimization and do AcceptInvalidationMessages always, but I think that cure would be a great deal worse than the disease --- it would hugely increase the contention for SInvalLock. I'm not particularly worried about missing a potential improvement in the plan during the first command after a change is committed. If the invalidation were something that *had* to be accounted for, such as a dropped index, then there should be adequate locking for it; plancache is not introducing any new bug that wasn't there before. regards, tom lane
On 4/3/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Me too. Just noticed it, so brought it up.
I'm not particularly worried about missing a potential improvement
in the plan during the first command after a change is committed.
Me too. Just noticed it, so brought it up.
If the invalidation were something that *had* to be accounted for,
such as a dropped index, then there should be adequate locking for it;
plancache is not introducing any new bug that wasn't there before.
Oh yes, I was wondering about the other parts of the code, not
plan invalidation. Never mind, it was just a thought.
Thanks,
Pavan
--
EnterpriseDB http://www.enterprisedb.com
"Pavan Deolasee" <pavan.deolasee@gmail.com> writes: > On 4/3/07, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> If the invalidation were something that *had* to be accounted for, >> such as a dropped index, then there should be adequate locking for it; >> plancache is not introducing any new bug that wasn't there before. >> > Oh yes, I was wondering about the other parts of the code, not > plan invalidation. Never mind, it was just a thought. Well, as that comment notes, we've always had to worry about being sure that the relcache data structures are up-to-date (or sufficiently up-to-date, anyway). I think it's reasonably well debugged. regards, tom lane