Thread: plpgsql memory leak in 7.3.2? (repost)
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.
"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);
> > 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.