SEO

Hi all,

We have a problem with an application which uses heavily ref cursors
to fetch results from the database.

We often have the following case:
- BEGIN
- lots of queries using ref cursors (it can run for an hour for
example). We close each result set when we don't use it anymore
- COMMIT

Even if we close the result sets, the memory of the PostgreSQL backend
is not released until the transaction is commited at the end of the
script. This is really a problem for us as PostgreSQL starts swapping
during the transaction.

The code used to manipulate the result set is:
CallableStatement myCallableStatement = myConnexion.prepareCall("{? =
Call cp_TestPostgresStandAlone()}");

myCallableStatement.registerOutParameter(1, Types.OTHER);

myCallableStatement.execute();



ResultSet myResultSet = (ResultSet) myCallableStatement.getObject(1);





/* simple manipulations of the result set */

myResultSet.close();

myCallableStatement.close();

After this two close() calls, the memory used by the cursor should be
released and it's not the case.

The stored proc is something like:
CREATE OR REPLACE FUNCTION cp_TestPostgresStandAlone()

RETURNS refcursor AS

$BODY$

DECLARE stock_cursor refcursor;

BEGIN

OPEN stock_cursor FOR

SELECT COUNT(*) FROM TestPostgresStandAlone;



RETURN stock_cursor;

END;

$BODY$

LANGUAGE 'plpgsql' VOLATILE;

AFAICS in the source code, the refcursor of a result set is not closed
when we close the result set and I don't think PostgreSQL closes it by
itself before the final commit.

Is there any way to fix this behaviour?

Thierry (in CC wrote a self contained test case to reproduce this
memory leak. We can post it to the list if necessary.

Thanks for your help.

--
Guillaume

---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Just a post to answer to the question of Dave Cramer on IRC and have a
comprehensive thread:
23:07 <dcramer> how would it know the cursor was open ?

AFAICS there's code in the driver to deal with the refcursor,
especially to get the cursor name and fetch all the results to a
result set.
Isn't there a way to keep the cursor name and close it automatically
when the statement or the resulting result set is closed?

I don't see an easy way to do it in the application directly (but any
tip is welcome) and ISTM it should be dealt by the driver directly to
prevent the memory usage of the backend growing dramatically during
the transaction.

--
Guillaume

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

"Guillaume Smet" <guillaume.smet@gmail.com> writes:
> Isn't there a way to keep the cursor name and close it automatically
> when the statement or the resulting result set is closed?

While I agree that the driver probably should do that, I'm not sure why
you are having a problem. A cursor doesn't normally accumulate its
result set in memory on the backend side, so I wouldn't have expected
all that big a memory leak. What queries are you executing?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Hi Tom,

On 1/17/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> While I agree that the driver probably should do that, I'm not sure why
> you are having a problem. A cursor doesn't normally accumulate its
> result set in memory on the backend side, so I wouldn't have expected
> all that big a memory leak. What queries are you executing?

I don't know exactly how much queries are executed during this
transaction but it's a lot of queries and they are _all_ executed
through stored proc returning cursors.

The self contained test case is available here:
http://people.openwide.fr/~gsmet/pos...ursor_problem/ and
especially the stored proc is in the readme file along with all the
instructions to run the test case.

I just run a test case with 10k iterations and the backend which
executes the queries uses 236 MB of memory before the commit.

--
Guillaume

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Tom,

The driver ends up issuing a

fetch all in <unnamed portal 1>

which if I try to close gives an error

cursor "<unnamed portal 1>" does not exist

so my guess is the portal is gone... what else might be keeping
server memory ???

I thought about this and long running transactions and if this were
really a problem slony would certainly blow up, as it has
transactions that run days.

Dave
On 17-Jan-07, at 5:31 PM, Tom Lane wrote:

> "Guillaume Smet" <guillaume.smet@gmail.com> writes:
>> Isn't there a way to keep the cursor name and close it automatically
>> when the statement or the resulting result set is closed?
>
> While I agree that the driver probably should do that, I'm not sure
> why
> you are having a problem. A cursor doesn't normally accumulate its
> result set in memory on the backend side, so I wouldn't have expected
> all that big a memory leak. What queries are you executing?
>
> regards, tom lane
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 6: explain analyze is your friend
>

Hi all,

Summary of our discussion with Dave on IRC.

The following patch provided by Dave fixed the problem for the first
31 iterations of our test but failed after that because the garbage
collector calls the close() method a second time when the object is
dereferenced (finalize() method of AbstractJDBC2Statement). So we have
to be sure that when the close() method is called a second time, it
doesn't fail.

On 1/18/07, Dave Cramer <davec@postgresintl.com> wrote:
> +
> + if ( this.refCursorName != null )
> + {
> + connection.execSQLUpdate("close \"" + refCursorName +'"');

I added a refCursorName = null; here to be sure the close cursor is
not executed twice and it works correctly.

> + }

I don't have the memory leaks anymore. I'll ask the people developing
the real application to test this patch to be sure everything is now
OK and I'll post the confirmation here.

Dave, thanks a lot for your help.

--
Guillaume

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Hi again,

In fact, there is still a remaining problem. People who developed this
application sometimes commit the transaction before closing the result
set in their code.
With the current driver, it's not a problem as it didn't close the
cursor at all so the cursor is closed at the end of the transaction
and that's all.

But with this patch, we have the following case:
- begin
- open result set -> open the cursor
- commit -> close the cursor
- close the result set -> try to close the cursor -> exception and
backend in an error state if autocommit is false

They fixed the order of commit/close in their application but IMHO,
it's a bad idea to introduce this sort of regression.

A try/catch is probably not a good idea because it leaves the backend
in an error state if the CLOSE query is executed in a transaction (if
autocommit is false for example).

Any ideas on how we can solve this problem? I can't find any way to
check if a cursor is still alive without throwing an error.

--
Guillaume

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

On 18-Jan-07, at 12:42 PM, Guillaume Smet wrote:

> Hi again,
>
> In fact, there is still a remaining problem. People who developed this
> application sometimes commit the transaction before closing the result
> set in their code.
> With the current driver, it's not a problem as it didn't close the
> cursor at all so the cursor is closed at the end of the transaction
> and that's all.
>
> But with this patch, we have the following case:
> - begin
> - open result set -> open the cursor
> - commit -> close the cursor
> - close the result set -> try to close the cursor -> exception and
> backend in an error state if autocommit is false
>
> They fixed the order of commit/close in their application but IMHO,
> it's a bad idea to introduce this sort of regression.
>
> A try/catch is probably not a good idea because it leaves the backend
> in an error state if the CLOSE query is executed in a transaction (if
> autocommit is false for example).
>
> Any ideas on how we can solve this problem? I can't find any way to
> check if a cursor is still alive without throwing an error.

There's one important piece of information I believe might be
relevant here.

The application is using XA. I'm not convinced that there isn't a
subtle bug in the server's XA implementation that may be keeping
track of the open cursor.

Under the covers this is using the unnamed cursor, and there is no
close cursor being issued, The cursor is implicitly closed by opening
another unnamed cursor.

I've asked Guillaume to test this hypothesis with his test case that
does not use XA to see if the memory "leak" still exists without XA

Dave
>
> --
> Guillaume
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that
> your
> message can get through to the mailing list cleanly
>


---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Hi Dave,

On 1/19/07, Dave Cramer <pg@fastcrypt.com> wrote:
> I've asked Guillaume to test this hypothesis with his test case that
> does not use XA to see if the memory "leak" still exists without XA

Perhaps I was not clear when I explained the remaining problem. I
don't have any memory leak left. The attached file is a test case
which shows the new problem.

The commit is done before closing the resultset and so the close()
method can't find the portal which results in an exception (the portal
is closed on commit).
It's probably not something we should do but before there was no error
in this case and now it throws an exception which is not really clear
for the end user:
An exception has occured.ERROR: cursor "<unnamed portal 1>" does not exist
org.postgresql.util.PSQLException: ERROR: cursor "<unnamed portal 1>"
does not exist

--
Guillaume

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Hi Guilluame

On 19-Jan-07, at 8:26 AM, Guillaume Smet wrote:

> Hi Dave,
>
> On 1/19/07, Dave Cramer <pg@fastcrypt.com> wrote:
>> I've asked Guillaume to test this hypothesis with his test case that
>> does not use XA to see if the memory "leak" still exists without XA
>
> Perhaps I was not clear when I explained the remaining problem. I
> don't have any memory leak left. The attached file is a test case
> which shows the new problem.

You were exceptionally clear when you described the problem you are
now having. Apparently I have not been clear.

I don't believe the solution is for the result set to close the
unnamed portal when it is closed. As you can see there is a catch-22
situation which you described clearly.

I believe the problem is in the server's XA code somehow not closing
unnamed parameters properly.

If you could run the code I sent you and tell me if it causes a leak,
then that will confirm it.

Dave

>
> The commit is done before closing the resultset and so the close()
> method can't find the portal which results in an exception (the portal
> is closed on commit).
> It's probably not something we should do but before there was no error
> in this case and now it throws an exception which is not really clear
> for the end user:
> An exception has occured.ERROR: cursor "<unnamed portal 1>" does
> not exist
> org.postgresql.util.PSQLException: ERROR: cursor "<unnamed portal 1>"
> does not exist
>
> --
> Guillaume
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 6: explain analyze is your friend
>


---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly