Обсуждение: Out-of-Memory with ROWTYPE assignment in PL/pgSQL FOR loop

Hello,

The PL/pgSQL FOR loop in the function consume_memory() defined below
will consume VM on each iteration until the process hits its ulimit.
The problem occurs with variables of ROWTYPE; there is no unbounded
allocation when using simple types such as integer or varchar.  Before I
delve into the SPI code, perhaps someone familiar with PostgreSQL internals
can spot the resource leak.

Tested with 8.0.1 and CVS head as of 2005-06-20:

-- Start of test code

-- create a table with ten million rows
CREATE TEMPORARY TABLE ten ( n integer DEFAULT 0 ) ;
INSERT INTO ten VALUES (0);
INSERT INTO ten VALUES (1);
INSERT INTO ten VALUES (2);
INSERT INTO ten VALUES (3);
INSERT INTO ten VALUES (4);
INSERT INTO ten VALUES (5);
INSERT INTO ten VALUES (6);
INSERT INTO ten VALUES (7);
INSERT INTO ten VALUES (8);
INSERT INTO ten VALUES (9);
CREATE TEMPORARY TABLE thousand AS
   SELECT 100*i.n + 10*j.n + k.n AS n FROM ten AS i, ten as j, ten as k ;
CREATE TEMPORARY TABLE tenmillion AS
   SELECT 10000*i.n + 10*j.n + k.n AS n FROM thousand AS i, thousand as j, ten as k ;

-- a function to consume memory
CREATE OR REPLACE FUNCTION consume_memory()
RETURNS void AS $PROC$
DECLARE
   rec tenmillion%ROWTYPE ;
   prev tenmillion%ROWTYPE ;
BEGIN
   FOR rec IN SELECT * FROM tenmillion LOOP
      prev := rec ;
   END LOOP ;
   RETURN ;
END
$PROC$ LANGUAGE plpgsql;

-- Until this point, memory usage is approximately constant.
-- Evaluating the above function will rapidly consume VM.

SELECT consume_memory() ;

-- End of test code

Here's a record of Committed_AS from /proc/meminfo on a Linux 2.6 system,
over the course of the test:

rugolsky@ti64: while : ; do grep Committed_AS /proc/meminfo ; sleep 1 ; done
Committed_AS:   225592 kB
Committed_AS:   225592 kB
Committed_AS:   233692 kB <- Started
Committed_AS:   258280 kB
Committed_AS:   282868 kB
Committed_AS:   299260 kB
Committed_AS:   323848 kB
Committed_AS:   340232 kB
Committed_AS:   348436 kB
Committed_AS:   356632 kB
Committed_AS:   381220 kB
Committed_AS:   397612 kB
Committed_AS:   414004 kB
Committed_AS:   422200 kB
Committed_AS:   438592 kB
Committed_AS:   463180 kB
Committed_AS:   487768 kB
Committed_AS:   504160 kB
Committed_AS:   504160 kB
Committed_AS:   520552 kB
Committed_AS:   545140 kB
Committed_AS:   569728 kB
Committed_AS:   586120 kB
Committed_AS:   586120 kB
Committed_AS:   602512 kB
Committed_AS:   225640 kB <- Cancelled

Regards,

    Bill Rugolsky

"Bill Rugolsky Jr." <brugolsky@telemetry-investments.com> writes:
> The PL/pgSQL FOR loop in the function consume_memory() defined below
> will consume VM on each iteration until the process hits its ulimit.
> The problem occurs with variables of ROWTYPE; there is no unbounded
> allocation when using simple types such as integer or varchar.

Yeah, looks like I introduced a memory leak with the 8.0 changes for
better support of rowtype variables :-(.  Here's the patch.

            regards, tom lane


Index: pl_exec.c
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.127.4.2
diff -c -r1.127.4.2 pl_exec.c
*** pl_exec.c    20 Jun 2005 20:44:50 -0000    1.127.4.2
--- pl_exec.c    20 Jun 2005 22:46:14 -0000
***************
*** 2003,2013 ****
      estate->eval_tuptable = NULL;
      estate->eval_processed = 0;
      estate->eval_lastoid = InvalidOid;
-     estate->eval_econtext = NULL;

      estate->err_func = func;
      estate->err_stmt = NULL;
      estate->err_text = NULL;
  }

  /* ----------
--- 2003,2032 ----
      estate->eval_tuptable = NULL;
      estate->eval_processed = 0;
      estate->eval_lastoid = InvalidOid;

      estate->err_func = func;
      estate->err_stmt = NULL;
      estate->err_text = NULL;
+
+     /*
+      * Create an EState for evaluation of simple expressions, if there's
+      * not one already in the current transaction.    The EState is made a
+      * child of TopTransactionContext so it will have the right lifespan.
+      */
+     if (simple_eval_estate == NULL)
+     {
+         MemoryContext oldcontext;
+
+         oldcontext = MemoryContextSwitchTo(TopTransactionContext);
+         simple_eval_estate = CreateExecutorState();
+         MemoryContextSwitchTo(oldcontext);
+     }
+
+     /*
+      * Create an expression context for simple expressions.
+      * This must be a child of simple_eval_estate.
+      */
+     estate->eval_econtext = CreateExprContext(simple_eval_estate);
  }

  /* ----------
***************
*** 3238,3243 ****
--- 3257,3264 ----
                  Datum *value,
                  bool *isnull)
  {
+     MemoryContext oldcontext;
+
      switch (datum->dtype)
      {
          case PLPGSQL_DTYPE_VAR:
***************
*** 3264,3272 ****
--- 3285,3295 ----
                      elog(ERROR, "row variable has no tupdesc");
                  /* Make sure we have a valid type/typmod setting */
                  BlessTupleDesc(row->rowtupdesc);
+                 oldcontext = MemoryContextSwitchTo(estate->eval_econtext->ecxt_per_tuple_memory);
                  tup = make_tuple_from_row(estate, row, row->rowtupdesc);
                  if (tup == NULL)    /* should not happen */
                      elog(ERROR, "row not compatible with its own tupdesc");
+                 MemoryContextSwitchTo(oldcontext);
                  *typeid = row->rowtupdesc->tdtypeid;
                  *value = HeapTupleGetDatum(tup);
                  *isnull = false;
***************
*** 3299,3308 ****
--- 3322,3333 ----
                   * fields. Copy the tuple body and insert the right
                   * values.
                   */
+                 oldcontext = MemoryContextSwitchTo(estate->eval_econtext->ecxt_per_tuple_memory);
                  heap_copytuple_with_tuple(rec->tup, &worktup);
                  HeapTupleHeaderSetDatumLength(worktup.t_data, worktup.t_len);
                  HeapTupleHeaderSetTypeId(worktup.t_data, rec->tupdesc->tdtypeid);
                  HeapTupleHeaderSetTypMod(worktup.t_data, rec->tupdesc->tdtypmod);
+                 MemoryContextSwitchTo(oldcontext);
                  *typeid = rec->tupdesc->tdtypeid;
                  *value = HeapTupleGetDatum(&worktup);
                  *isnull = false;
***************
*** 3579,3585 ****
                        Oid *rettype)
  {
      Datum        retval;
!     ExprContext * volatile econtext;
      ParamListInfo paramLI;
      int            i;
      Snapshot    saveActiveSnapshot;
--- 3604,3610 ----
                        Oid *rettype)
  {
      Datum        retval;
!     ExprContext *econtext = estate->eval_econtext;
      ParamListInfo paramLI;
      int            i;
      Snapshot    saveActiveSnapshot;
***************
*** 3590,3609 ****
      *rettype = expr->expr_simple_type;

      /*
-      * Create an EState for evaluation of simple expressions, if there's
-      * not one already in the current transaction.    The EState is made a
-      * child of TopTransactionContext so it will have the right lifespan.
-      */
-     if (simple_eval_estate == NULL)
-     {
-         MemoryContext oldcontext;
-
-         oldcontext = MemoryContextSwitchTo(TopTransactionContext);
-         simple_eval_estate = CreateExecutorState();
-         MemoryContextSwitchTo(oldcontext);
-     }
-
-     /*
       * Prepare the expression for execution, if it's not been done already
       * in the current transaction.
       */
--- 3615,3620 ----
***************
*** 3617,3634 ****
      }

      /*
-      * Create an expression context for simple expressions, if there's not
-      * one already in the current function call.  This must be a child of
-      * simple_eval_estate.
-      */
-     econtext = estate->eval_econtext;
-     if (econtext == NULL)
-     {
-         econtext = CreateExprContext(simple_eval_estate);
-         estate->eval_econtext = econtext;
-     }
-
-     /*
       * Param list can live in econtext's temporary memory context.
       *
       * XXX think about avoiding repeated palloc's for param lists? Beware
--- 3628,3633 ----

On Mon, Jun 20, 2005 at 06:54:20PM -0400, Tom Lane wrote:
> "Bill Rugolsky Jr." <brugolsky@telemetry-investments.com> writes:
> > The PL/pgSQL FOR loop in the function consume_memory() defined below
> > will consume VM on each iteration until the process hits its ulimit.
> > The problem occurs with variables of ROWTYPE; there is no unbounded
> > allocation when using simple types such as integer or varchar.
>
> Yeah, looks like I introduced a memory leak with the 8.0 changes for
> better support of rowtype variables :-(.  Here's the patch.

Thank you for the quick reply; much appreciated!
Applied and tested against CVS head; that plugged the leak.

    -Bill