plpgsql-stmt_call-fix-2.patch

text/x-patch

Filename: plpgsql-stmt_call-fix-2.patch
Type: text/x-patch
Part: 0
Message: Re: calling procedures is slow and consumes extra much memory against calling function

Patch

Format: unified
File+
src/backend/executor/spi.c 37 4
src/backend/utils/cache/plancache.c 25 1
src/backend/utils/resowner/resowner.c 8 2
src/include/executor/spi_priv.h 1 0
src/include/utils/plancache.h 3 0
src/pl/plpgsql/src/pl_exec.c 10 1
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 055ebb77ae..d6bbbc3dbd 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -24,6 +24,7 @@
 #include "executor/executor.h"
 #include "executor/spi_priv.h"
 #include "miscadmin.h"
+#include "storage/proc.h"
 #include "tcop/pquery.h"
 #include "tcop/utility.h"
 #include "utils/builtins.h"
@@ -2223,6 +2224,9 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
 	CachedPlan *cplan = NULL;
 	ListCell   *lc1;
 
+	LocalTransactionId before_lxid = MyProc->lxid;
+	bool		is_fragile = plan->fragile;
+
 	/*
 	 * Setup error traceback support for ereport()
 	 */
@@ -2326,6 +2330,8 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
 		 * plan, the refcount must be backed by the CurrentResourceOwner.
 		 */
 		cplan = GetCachedPlan(plansource, paramLI, plan->saved, _SPI_current->queryEnv);
+		cplan->is_fragile = is_fragile;
+
 		stmt_list = cplan->stmt_list;
 
 		/*
@@ -2520,8 +2526,19 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
 			}
 		}
 
-		/* Done with this plan, so release refcount */
-		ReleaseCachedPlan(cplan, plan->saved);
+		if (is_fragile)
+		{
+			LocalTransactionId after_lxid  = MyProc->lxid;;
+
+			if (before_lxid == after_lxid)
+				ReleaseCachedPlan(cplan, plan->saved);
+
+			FinalReleaseFragileCachedPlan(cplan);
+		}
+		else
+			/* Done with this plan, so release refcount */
+			ReleaseCachedPlan(cplan, plan->saved);
+
 		cplan = NULL;
 
 		/*
@@ -2539,9 +2556,25 @@ fail:
 	if (pushed_active_snap)
 		PopActiveSnapshot();
 
-	/* We no longer need the cached plan refcount, if any */
 	if (cplan)
-		ReleaseCachedPlan(cplan, plan->saved);
+	{
+		if (is_fragile)
+		{
+			LocalTransactionId after_lxid  = MyProc->lxid;;
+
+			/*
+			 * Release plan, when the transaction is same. When
+			 * there are new transaction, then cached plan was
+			 * released by resource owner.
+			 */
+			if (before_lxid == after_lxid)
+				ReleaseCachedPlan(cplan, plan->saved);
+
+			FinalReleaseFragileCachedPlan(cplan);
+		}
+		else
+			ReleaseCachedPlan(cplan, plan->saved);
+	}
 
 	/*
 	 * Pop the error context stack
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 75b475c179..de3ed036f2 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -528,10 +528,14 @@ ReleaseGenericPlan(CachedPlanSource *plansource)
 	if (plansource->gplan)
 	{
 		CachedPlan *plan = plansource->gplan;
+		bool		is_fragile = plan->is_fragile;
 
 		Assert(plan->magic == CACHEDPLAN_MAGIC);
 		plansource->gplan = NULL;
 		ReleaseCachedPlan(plan, false);
+
+		if (is_fragile)
+			FinalReleaseFragileCachedPlan(plan);
 	}
 }
 
@@ -1265,9 +1269,12 @@ ReleaseCachedPlan(CachedPlan *plan, bool useResOwner)
 		Assert(plan->is_saved);
 		ResourceOwnerForgetPlanCacheRef(CurrentResourceOwner, plan);
 	}
+
 	Assert(plan->refcount > 0);
 	plan->refcount--;
-	if (plan->refcount == 0)
+
+	/* We don't want to destroy fragile plans immediately */
+	if (plan->refcount == 0 && !plan->is_fragile)
 	{
 		/* Mark it no longer valid */
 		plan->magic = 0;
@@ -1278,6 +1285,23 @@ ReleaseCachedPlan(CachedPlan *plan, bool useResOwner)
 	}
 }
 
+void
+FinalReleaseFragileCachedPlan(CachedPlan *plan)
+{
+
+	Assert(plan->magic == CACHEDPLAN_MAGIC);
+	Assert(plan->is_fragile);
+
+	if (plan->refcount == 0)
+	{
+		plan->magic = 0;
+
+		/* One-shot plans do not own their context, so we can't free them */
+		if (!plan->is_oneshot)
+			MemoryContextDelete(plan->context);
+	}
+}
+
 /*
  * CachedPlanAllowsSimpleValidityCheck: can we use CachedPlanIsSimplyValid?
  *
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 8bc2c4e9ea..b328729ebd 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -635,7 +635,13 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 		{
 			CachedPlan *res = (CachedPlan *) DatumGetPointer(foundres);
 
-			if (isCommit)
+			/*
+			 * The transaction can be ended inside execution of CALL statements.
+			 * In this case, the caller cannot to clean own resources, and then
+			 * cleaning by resource owner is expected. Unfortunatelly a caller
+			 * routine doesn't know if this situation will be or not.
+			 */
+			if (isCommit && !res->is_fragile)
 				PrintPlanCacheLeakWarning(res);
 			ReleaseCachedPlan(res, true);
 		}
@@ -1140,7 +1146,7 @@ ResourceOwnerRememberPlanCacheRef(ResourceOwner owner, CachedPlan *plan)
 void
 ResourceOwnerForgetPlanCacheRef(ResourceOwner owner, CachedPlan *plan)
 {
-	if (!ResourceArrayRemove(&(owner->planrefarr), PointerGetDatum(plan)))
+	if (!ResourceArrayRemove(&(owner->planrefarr), PointerGetDatum(plan)) && !plan->is_fragile)
 		elog(ERROR, "plancache reference %p is not owned by resource owner %s",
 			 plan, owner->name);
 }
diff --git a/src/include/executor/spi_priv.h b/src/include/executor/spi_priv.h
index 6220928bd3..e1a01ecc0b 100644
--- a/src/include/executor/spi_priv.h
+++ b/src/include/executor/spi_priv.h
@@ -93,6 +93,7 @@ typedef struct _SPI_plan
 	bool		saved;			/* saved or unsaved plan? */
 	bool		oneshot;		/* one-shot plan? */
 	bool		no_snapshots;	/* let the caller handle the snapshots */
+	bool		fragile;		/* plancache can be released by resource owner */
 	List	   *plancache_list; /* one CachedPlanSource per parsetree */
 	MemoryContext plancxt;		/* Context containing _SPI_plan and data */
 	int			cursor_options; /* Cursor options used for planning */
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 522020d763..d657eb0f17 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -150,6 +150,7 @@ typedef struct CachedPlan
 	bool		is_oneshot;		/* is it a "oneshot" plan? */
 	bool		is_saved;		/* is CachedPlan in a long-lived context? */
 	bool		is_valid;		/* is the stmt_list currently valid? */
+	bool		is_fragile;		/* cleaning by resource owner is expected */
 	Oid			planRoleId;		/* Role ID the plan was created for */
 	bool		dependsOnRole;	/* is plan specific to that role? */
 	TransactionId saved_xmin;	/* if valid, replan when TransactionXmin
@@ -222,6 +223,8 @@ extern CachedPlan *GetCachedPlan(CachedPlanSource *plansource,
 								 QueryEnvironment *queryEnv);
 extern void ReleaseCachedPlan(CachedPlan *plan, bool useResOwner);
 
+extern void FinalReleaseFragileCachedPlan(CachedPlan *plan);
+
 extern bool CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource,
 												CachedPlan *plan,
 												ResourceOwner owner);
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index d4a3d58daa..0f2ba6b0f6 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2171,7 +2171,7 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
 			 * XXX This would be fixable with some plancache/resowner surgery
 			 * elsewhere, but for now we'll just work around this here.
 			 */
-			exec_prepare_plan(estate, expr, 0, estate->atomic);
+			exec_prepare_plan(estate, expr, 0, true);
 
 			/*
 			 * The procedure call could end transactions, which would upset
@@ -2181,6 +2181,15 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
 			plan = expr->plan;
 			plan->no_snapshots = true;
 
+			/*
+			 * Plancache have to be released on end of tranaction. But for this
+			 * case, transaction can be ended somewhere inside CALL statements
+			 * chain. We cannot to know where and why. So we should to mark
+			 * this plan as fragile. In this case is possible, there are
+			 * more actors are responsible to cleaning plan cache.
+			 */
+			plan->fragile = true;
+
 			/*
 			 * Force target to be recalculated whenever the plan changes, in
 			 * case the procedure's argument list has changed.