From 40b963d6496a42a71ef992f55963c1e77a7bb65b Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Thu, 21 Oct 2010 14:17:54 -0300 Subject: [PATCH] Mark a cache plan as dead when aborting its creating subtransaction. Per a trouble report from Enova Financial's Jim Nasby and Jon Erdman. --- src/backend/commands/prepare.c | 6 +++--- src/backend/executor/spi.c | 6 +++--- src/backend/tcop/postgres.c | 4 ++-- src/backend/utils/cache/plancache.c | 10 ++++++---- src/backend/utils/mmgr/portalmem.c | 2 +- src/backend/utils/resowner/resowner.c | 2 +- src/include/utils/plancache.h | 2 +- src/pl/plpgsql/src/pl_exec.c | 4 ++-- 8 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c index c30ce35..8fb6b3c 100644 --- a/src/backend/commands/prepare.c +++ b/src/backend/commands/prepare.c @@ -250,7 +250,7 @@ ExecuteQuery(ExecuteStmt *stmt, const char *queryString, MemoryContextSwitchTo(oldContext); /* We no longer need the cached plan refcount ... */ - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, false); /* ... and we don't want the portal to depend on it, either */ cplan = NULL; } @@ -568,7 +568,7 @@ FetchPreparedStatementTargetList(PreparedStatement *stmt) /* Copy into caller's context so we can release the plancache entry */ tlist = (List *) copyObject(tlist); - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, false); return tlist; } @@ -726,7 +726,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, ExplainStmt *stmt, if (estate) FreeExecutorState(estate); - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, false); } /* diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 3cffc2a..d826236 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -1384,7 +1384,7 @@ SPI_is_cursor_plan(SPIPlanPtr plan) { /* Make sure the plan is up to date */ cplan = RevalidateCachedPlan(plansource, true); - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, false); } _SPI_end_call(false); @@ -1878,7 +1878,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, /* Done with this plan, so release refcount */ if (cplan) - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, false); cplan = NULL; /* @@ -1894,7 +1894,7 @@ fail: /* We no longer need the cached plan refcount, if any */ if (cplan) - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, false); /* * Pop the error context stack diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 43e912f..e7c798d 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -1709,7 +1709,7 @@ exec_bind_message(StringInfo input_message) MemoryContextSwitchTo(oldContext); /* We no longer need the cached plan refcount ... */ - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, false); /* ... and we don't want the portal to depend on it, either */ cplan = NULL; } @@ -2266,7 +2266,7 @@ exec_describe_statement_message(const char *stmt_name) SendRowDescriptionMessage(psrc->resultDesc, tlist, NULL); - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, false); } else pq_putemptymessage('n'); /* NoData */ diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index 6c3082d..338753e 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -367,7 +367,7 @@ DropCachedPlan(CachedPlanSource *plansource) /* Decrement child CachePlan's refcount and drop if no longer needed */ if (plansource->plan) - ReleaseCachedPlan(plansource->plan, false); + ReleaseCachedPlan(plansource->plan, false, false); /* * If CachedPlanSource has independent storage, just drop it. Otherwise @@ -381,7 +381,7 @@ DropCachedPlan(CachedPlanSource *plansource) else { Assert(plansource->context == plansource->orig_plan->context); - ReleaseCachedPlan(plansource->orig_plan, false); + ReleaseCachedPlan(plansource->orig_plan, false, false); } } @@ -455,7 +455,7 @@ RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner) if (plan && plan->dead) { plansource->plan = NULL; - ReleaseCachedPlan(plan, false); + ReleaseCachedPlan(plan, false, false); plan = NULL; } @@ -588,12 +588,14 @@ RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner) * Portal. Transient references should be protected by a resource owner. */ void -ReleaseCachedPlan(CachedPlan *plan, bool useResOwner) +ReleaseCachedPlan(CachedPlan *plan, bool useResOwner, bool isabort) { if (useResOwner) ResourceOwnerForgetPlanCacheRef(CurrentResourceOwner, plan); Assert(plan->refcount > 0); plan->refcount--; + if (isabort) + plan->dead = true; if (plan->refcount == 0) MemoryContextDelete(plan->context); } diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c index 0445705..e01b3b0 100644 --- a/src/backend/utils/mmgr/portalmem.c +++ b/src/backend/utils/mmgr/portalmem.c @@ -326,7 +326,7 @@ PortalReleaseCachedPlan(Portal portal) { if (portal->cplan) { - ReleaseCachedPlan(portal->cplan, false); + ReleaseCachedPlan(portal->cplan, false, false); portal->cplan = NULL; /* diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c index 9332678..c0cd77b 100644 --- a/src/backend/utils/resowner/resowner.c +++ b/src/backend/utils/resowner/resowner.c @@ -305,7 +305,7 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, { if (isCommit) PrintPlanCacheLeakWarning(owner->planrefs[owner->nplanrefs - 1]); - ReleaseCachedPlan(owner->planrefs[owner->nplanrefs - 1], true); + ReleaseCachedPlan(owner->planrefs[owner->nplanrefs - 1], true, !isCommit); } /* Ditto for tupdesc references */ while (owner->ntupdescs > 0) diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h index ea919bd..1be054d 100644 --- a/src/include/utils/plancache.h +++ b/src/include/utils/plancache.h @@ -108,7 +108,7 @@ extern CachedPlanSource *FastCreateCachedPlan(Node *raw_parse_tree, extern void DropCachedPlan(CachedPlanSource *plansource); extern CachedPlan *RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner); -extern void ReleaseCachedPlan(CachedPlan *plan, bool useResOwner); +extern void ReleaseCachedPlan(CachedPlan *plan, bool useResOwner, bool isabort); extern bool CachedPlanIsValid(CachedPlanSource *plansource); extern TupleDesc PlanCacheComputeResultDesc(List *stmt_list); diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 8d0c625..6d5d729 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -4435,7 +4435,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, if (expr->expr_simple_expr == NULL) { /* Ooops, release refcount and fail */ - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, false); return false; } } @@ -4525,7 +4525,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, /* * Now we can release our refcount on the cached plan. */ - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, false); /* * That's it. -- 1.7.1