Thread: plpgsql memory leak in 7.3.2? (repost)

plpgsql memory leak in 7.3.2? (repost)

From
"Mark Cave-Ayland"
Date:
Not sure this made it to the list the first time around, apologies to
anyone who received it twice.


Mark.


-----Original Message-----
From: Mark Cave-Ayland [mailto:m.cave-ayland@webbased.co.uk]
Sent: 01 March 2003 21:10
To: 'pgsql-general@postgresql.org'
Subject: plpgsql memory leak in 7.3.2?


Hi everyone,

Whilst continuing work on some of our large tables again, we've
encountered what we think may be a memory leak in plpgsql in 7.3.2. The
process to recreate the problem we are experiencing is given below:


1. First create a table with a million or so rows and create an index on
it

create table restable (
    resid int8
    );

create or replace function t_pop() returns int as '
declare
    i int8;
    sql varchar;
begin
    i := 0;

    while i < 1000000 loop
        sql := ''insert into restable values ('' || i || '')'';
        execute sql;
        i:=i+1;
    end loop;

    return 1;
end;
' language 'plpgsql';

select t_pop();

create index restable_index on restable(resid);


2. Next create a simplified version of our stored procedure

create or replace function t_leak() returns int as '
declare
    count int8;
    row record;
    sql varchar;
begin
    count := 0;

    while count < 1000000 loop

        sql := ''select * from restable where resid='' || count
|| ''::bigint'';
        for row in execute sql loop
            -- Do something interesting here....
            -- Yes, the line below can still be commented
and the problem
            -- still occurs!
            -- execute sql;
        end loop;
        count := count + 1;
    end loop;

    return 1;
end;
' language 'plpgsql';


3. Connect to another session the machine running the db and execute
'top -d1' to view
   memory usage.

4. Start the stored procedure t_leak() and watch the memory usage


What we are finding happens here is that the postmaster process
continues to grow and grab megs of memory (approx 600Mb in this case)
even though it should only be retrieving one record at a time based on
an index. The problem seems to be caused by the 'for row in execute sql
loop' section, since when this is removed, the problem goes away.
However it is necessary to restart the database and then reinsert the
function before the memory usage goes back to normal.

We would be really grateful if someone could verify if this is a bug or
a problem with our build environment and suggest a suitable fix. What we
have done at the moment is to downgrade back to 7.3.1 which doesn't
appear to exhibit the same problem (memory usage remains minimal) so we
can run these stored procedures. On our production system, the memory
grabbed by the postmaster process is about 1.4G which, as you can
imagine, causes everything to run very slowly.....


Many thanks,


Mark.


---

Mark Cave-Ayland
Webbased Ltd.
Tamar Science Park
Derriford
Plymouth
PL6 8BX
England

Tel: +44 (0)1752 764445
Fax: +44 (0)1752 764446


This email and any attachments are confidential to the intended
recipient and may also be privileged. If you are not the intended
recipient please delete it from your system and notify the sender. You
should not copy it or use it for any purpose nor disclose or distribute
its contents to any other person.



Re: plpgsql memory leak in 7.3.2? (repost)

From
Tom Lane
Date:
"Mark Cave-Ayland" <m.cave-ayland@webbased.co.uk> writes:
> Whilst continuing work on some of our large tables again, we've
> encountered what we think may be a memory leak in plpgsql in 7.3.2.

Yup, you're right.  Looks like I introduced several memory leaks in
plpgsql when I modified spi.c to return a tuple descriptor even with
zero tuples returned: some plpgsql routines assumed they didn't need
to do SPI_freetuptable() after retrieving no tuples.  Patch attached,
or you can grab the updated pl_exec.c from our CVS server.

            regards, tom lane


*** src/pl/plpgsql/src/pl_exec.c~    Tue Jan 21 17:06:36 2003
--- src/pl/plpgsql/src/pl_exec.c    Sun Mar  2 15:45:59 2003
***************
*** 1369,1379 ****
              if (rc != PLPGSQL_RC_OK)
              {
                  /*
!                  * We're aborting the loop, so cleanup and set FOUND
                   */
-                 exec_set_found(estate, found);
                  SPI_freetuptable(tuptab);
                  SPI_cursor_close(portal);

                  if (rc == PLPGSQL_RC_EXIT)
                  {
--- 1369,1380 ----
              if (rc != PLPGSQL_RC_OK)
              {
                  /*
!                  * We're aborting the loop, so cleanup and set FOUND.
!                  * (This code should match the code after the loop.)
                   */
                  SPI_freetuptable(tuptab);
                  SPI_cursor_close(portal);
+                 exec_set_found(estate, found);

                  if (rc == PLPGSQL_RC_EXIT)
                  {
***************
*** 1411,1416 ****
--- 1412,1422 ----
      }

      /*
+      * Release last group of tuples
+      */
+     SPI_freetuptable(tuptab);
+
+     /*
       * Close the implicit cursor
       */
      SPI_cursor_close(portal);
***************
*** 2363,2369 ****
      if (n == 0)
          exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
      else
!         found = true;

      /*
       * Now do the loop
--- 2369,2375 ----
      if (n == 0)
          exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
      else
!         found = true;            /* processed at least one tuple */

      /*
       * Now do the loop
***************
*** 2382,2395 ****
               */
              rc = exec_stmts(estate, stmt->body);

-             /*
-              * We're aborting the loop, so cleanup and set FOUND
-              */
              if (rc != PLPGSQL_RC_OK)
              {
!                 exec_set_found(estate, found);
                  SPI_freetuptable(tuptab);
                  SPI_cursor_close(portal);

                  if (rc == PLPGSQL_RC_EXIT)
                  {
--- 2388,2402 ----
               */
              rc = exec_stmts(estate, stmt->body);

              if (rc != PLPGSQL_RC_OK)
              {
!                 /*
!                  * We're aborting the loop, so cleanup and set FOUND.
!                  * (This code should match the code after the loop.)
!                  */
                  SPI_freetuptable(tuptab);
                  SPI_cursor_close(portal);
+                 exec_set_found(estate, found);

                  if (rc == PLPGSQL_RC_EXIT)
                  {
***************
*** 2427,2433 ****
      }

      /*
!      * Close the cursor
       */
      SPI_cursor_close(portal);

--- 2434,2445 ----
      }

      /*
!      * Release last group of tuples
!      */
!     SPI_freetuptable(tuptab);
!
!     /*
!      * Close the implicit cursor
       */
      SPI_cursor_close(portal);

***************
*** 2736,2759 ****
      pfree(curname);

      /* ----------
-      * Initialize the global found variable to false
-      * ----------
-      */
-     exec_set_found(estate, false);
-
-     /* ----------
       * Determine if we fetch into a record or a row
       * ----------
       */
      if (stmt->rec != NULL)
          rec = (PLpgSQL_rec *) (estate->datums[stmt->rec->recno]);
      else
!     {
!         if (stmt->row != NULL)
!             row = (PLpgSQL_row *) (estate->datums[stmt->row->rowno]);
!         else
!             elog(ERROR, "unsupported target in exec_stmt_select()");
!     }

      /* ----------
       * Fetch 1 tuple from the cursor
--- 2748,2762 ----
      pfree(curname);

      /* ----------
       * Determine if we fetch into a record or a row
       * ----------
       */
      if (stmt->rec != NULL)
          rec = (PLpgSQL_rec *) (estate->datums[stmt->rec->recno]);
+     else if (stmt->row != NULL)
+         row = (PLpgSQL_row *) (estate->datums[stmt->row->rowno]);
      else
!         elog(ERROR, "unsupported target in exec_stmt_fetch()");

      /* ----------
       * Fetch 1 tuple from the cursor
***************
*** 2764,2785 ****
      n = SPI_processed;

      /* ----------
!      * If the FETCH didn't return a row, set the target
!      * to NULL and return with FOUND = false.
       * ----------
       */
      if (n == 0)
      {
          exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
!         return PLPGSQL_RC_OK;
      }
-
-     /* ----------
-      * Put the result into the target and set found to true
-      * ----------
-      */
-     exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc);
-     exec_set_found(estate, true);

      SPI_freetuptable(tuptab);

--- 2767,2785 ----
      n = SPI_processed;

      /* ----------
!      * Set the target and the global FOUND variable appropriately.
       * ----------
       */
      if (n == 0)
      {
          exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
!         exec_set_found(estate, false);
!     }
!     else
!     {
!         exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc);
!         exec_set_found(estate, true);
      }

      SPI_freetuptable(tuptab);


Re: plpgsql memory leak in 7.3.2? (repost)

From
"Mark Cave-Ayland"
Date:
> > Whilst continuing work on some of our large tables again, we've
> > encountered what we think may be a memory leak in plpgsql in 7.3.2.
>
> Yup, you're right.  Looks like I introduced several memory
> leaks in plpgsql when I modified spi.c to return a tuple
> descriptor even with zero tuples returned: some plpgsql
> routines assumed they didn't need to do SPI_freetuptable()
> after retrieving no tuples.  Patch attached, or you can grab
> the updated pl_exec.c from our CVS server.
>
>             regards, tom lane

Hi Tom,

Firstly I'd just like to say I am really impressed by the phenomenal
response time from posting the email to receiving a patch - absolutely
fantastic, especially on a Sunday! I've built up a new RPM to test the
patch and the test case from the email works brilliantly now. We'll
probably start tagging more data from large tables on our live system
over next weekend which will give me a chance to give it a through
testing, but as the email test case works I'm really confident that
you've managed to nail this one!


Once again, thank you very much,

Mark.


---

Mark Cave-Ayland
Webbased Ltd.
Tamar Science Park
Derriford
Plymouth
PL6 8BX
England

Tel: +44 (0)1752 764445
Fax: +44 (0)1752 764446


This email and any attachments are confidential to the intended
recipient and may also be privileged. If you are not the intended
recipient please delete it from your system and notify the sender. You
should not copy it or use it for any purpose nor disclose or distribute
its contents to any other person.