Обсуждение: Memory leaks using refcursors

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

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

"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

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/postgresql/refcursor_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

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

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

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
>

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

Outch, forgot the attachment...

--
Guillaume

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
>

On 1/19/07, Dave Cramer <pg@fastcrypt.com> wrote:
> 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.

Sorry, I did not understand what you've asked me. I just run the test
without the XA code and it has the memory leak with the old 408
driver.
IMHO, it's quite logical as this memory leak has nothing to do with
the transaction management as all the operations are in a big
transaction.
When I close the transaction, XA or not XA, the backend used by the
connection returns to a normal memory usage.

--
Guillaume

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

> On 1/19/07, Dave Cramer <pg@fastcrypt.com> wrote:
>> 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.
>
> Sorry, I did not understand what you've asked me. I just run the test
> without the XA code and it has the memory leak with the old 408
> driver.
> IMHO, it's quite logical as this memory leak has nothing to do with
> the transaction management as all the operations are in a big
> transaction.
> When I close the transaction, XA or not XA, the backend used by the
> connection returns to a normal memory usage.

So I've sent Guillaume a driver which closes the refcursor directly
after reading the contents for testing.

So far so good.

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

Hello,

Can someone explain why I cann't get bytea data of more than 1998 bytes?
Below 1998 size is ok but I got exception if the size is at and above
1998.  I used jdbc-8.1-407.jdbc3.jar.

//setting bytea value.  Work fine
byte[] bdata = new byte[1998];
                    for (int b = 0; b < bdata.length; b++) {
                        bdata[b] = (byte) (b % Byte.MAX_VALUE);
                    }

                    ByteArrayInputStream is = new
ByteArrayInputStream(bdata);

                    prepstmt.setBinaryStream(1, is, bdata.length);

//getting bytea value.  Got error
ByteArrayInputStream s = (ByteArrayInputStream) rs
                                .getBinaryStream(1);

Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException:
1998
    at org.postgresql.util.PGbytea.toBytes(PGbytea.java:39)
    at
org.postgresql.jdbc2.AbstractJdbc2ResultSet.getBytes(AbstractJdbc2Result
Set.java:2048)
    at
org.postgresql.jdbc2.AbstractJdbc2ResultSet.getBinaryStream(AbstractJdbc
2ResultSet.java:2168)

Thanks

Leon Do
> -----Original Message-----
> From: pgsql-jdbc-owner@postgresql.org
> [mailto:pgsql-jdbc-owner@postgresql.org] On Behalf Of Dave Cramer
> Sent: Friday, January 19, 2007 2:13 PM
> To: Guillaume Smet
> Cc: PostgreSQL JDBC; Tom Lane
> Subject: Re: [JDBC] Memory leaks using refcursors
>
>
> On 19-Jan-07, at 11:07 AM, Guillaume Smet wrote:
>
> > On 1/19/07, Dave Cramer <pg@fastcrypt.com> wrote:
> >> 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.
> >
> > Sorry, I did not understand what you've asked me. I just
> run the test
> > without the XA code and it has the memory leak with the old 408
> > driver.
> > IMHO, it's quite logical as this memory leak has nothing to do with
> > the transaction management as all the operations are in a big
> > transaction.
> > When I close the transaction, XA or not XA, the backend used by the
> > connection returns to a normal memory usage.
>
> So I've sent Guillaume a driver which closes the refcursor
> directly after reading the contents for testing.
>
> So far so good.
>
> Dave
> >
> > --
> > Guillaume
> >
> > ---------------------------(end of
> > broadcast)---------------------------
> > TIP 5: don't forget to increase your free space map settings
> >
>
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
>                http://archives.postgresql.org
>

On Fri, 19 Jan 2007, Do, Leon (Leon) wrote:

> Hello,
>

When starting a new thread, please don't reply to some other random
message on the list.

> Can someone explain why I cann't get bytea data of more than 1998 bytes?
> Below 1998 size is ok but I got exception if the size is at and above
> 1998.  I used jdbc-8.1-407.jdbc3.jar.
>

Could you provide a complete test case.  What you've written below isn't
entirely clear about what's going on.  I believe bytea decoding
is done correctly.  Perhaps you are doing something like calling
getBinaryStream on a column that is not bytea?

Kris Jurka

Sorry, it is my mistake.   It is wrong data type.

Thanks

Leon Do


> -----Original Message-----
> From: pgsql-jdbc-owner@postgresql.org
> [mailto:pgsql-jdbc-owner@postgresql.org] On Behalf Of Kris Jurka
> Sent: Sunday, January 21, 2007 12:10 AM
> To: Do, Leon (Leon)
> Cc: PostgreSQL JDBC
> Subject: Re: [JDBC] Max size for bytea type
>
>
>
> On Fri, 19 Jan 2007, Do, Leon (Leon) wrote:
>
> > Hello,
> >
>
> When starting a new thread, please don't reply to some other
> random message on the list.
>
> > Can someone explain why I cann't get bytea data of more
> than 1998 bytes?
> > Below 1998 size is ok but I got exception if the size is at
> and above
> > 1998.  I used jdbc-8.1-407.jdbc3.jar.
> >
>
> Could you provide a complete test case.  What you've written
> below isn't entirely clear about what's going on.  I believe
> bytea decoding is done correctly.  Perhaps you are doing
> something like calling getBinaryStream on a column that is not bytea?
>
> Kris Jurka
>
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
>                http://www.postgresql.org/docs/faq
>

On Mon, 22 Jan 2007, Do, Leon (Leon) wrote:

> Sorry, it is my mistake.   It is wrong data type.
>

Still there is a TODO item here.  The driver shouldn't bail out with an
ArrayIndexOutOfBoundsException.  It might be best to fail immediately for
a getBytes call on something that isn't bytea data.  It's conceivable that
you there is bytea escaped data in a text field, but really, what are the
odds (and use cases) of that?

Kris Jurka

Yes, we need a TODO item on the bytea parser.  We need to be able to
handle rainny scenarios when data is bad.

Thanks

Leon Do

> -----Original Message-----
> From: Kris Jurka [mailto:books@ejurka.com]
> Sent: Thursday, February 01, 2007 1:32 PM
> To: Do, Leon (Leon)
> Cc: PostgreSQL JDBC
> Subject: Re: [JDBC] Max size for bytea type
>
>
>
> On Mon, 22 Jan 2007, Do, Leon (Leon) wrote:
>
> > Sorry, it is my mistake.   It is wrong data type.
> >
>
> Still there is a TODO item here.  The driver shouldn't bail
> out with an ArrayIndexOutOfBoundsException.  It might be best
> to fail immediately for a getBytes call on something that
> isn't bytea data.  It's conceivable that you there is bytea
> escaped data in a text field, but really, what are the odds
> (and use cases) of that?
>
> Kris Jurka
>
>

On Thu, 1 Feb 2007, Kris Jurka wrote:

> On Mon, 22 Jan 2007, Do, Leon (Leon) wrote:
>
>> Sorry, it is my mistake.   It is wrong data type.
>>
>
> Still there is a TODO item here.  The driver shouldn't bail out with an
> ArrayIndexOutOfBoundsException.  It might be best to fail immediately for a
> getBytes call on something that isn't bytea data.  It's conceivable that you
> there is bytea escaped data in a text field, but really, what are the odds
> (and use cases) of that?
>

Checking the code behind this, I see:

AbstractJdbc2ResultSet.java:2150

     if (fields[columnIndex - 1].getOID() == Oid.BYTEA)
     {
         return trimBytes(columnIndex,PGbytea.toBytes(this_row[columnIndex - 1]));
     }
     else
     {
         return trimBytes(columnIndex, this_row[columnIndex - 1]);
     }

So the check I suggested already exists and I don't understand how you got
this error.  Do you still have your test case handy?

Kris Jurka

On Fri, 19 Jan 2007, Dave Cramer wrote:

> So I've sent Guillaume a driver which closes the refcursor directly after
> reading the contents for testing.
>
> So far so good.
>

So what's the resolution on this?  Should this fix be committed?  What are
the downsides?

Kris Jurka

Kris,

I don't see a downside to it. Once we read the data from the cursor
we have no more use for it. AFACT it should be committed.

Dave
On 18-Feb-07, at 7:20 PM, Kris Jurka wrote:

>
>
> On Fri, 19 Jan 2007, Dave Cramer wrote:
>
>> So I've sent Guillaume a driver which closes the refcursor
>> directly after reading the contents for testing.
>>
>> So far so good.
>>
>
> So what's the resolution on this?  Should this fix be committed?
> What are the downsides?
>
> Kris Jurka
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
>               http://archives.postgresql.org
>

Hi Kris,

I told Dave on IRC that it was OK for us. The memory leak is fixed and
we don't have any regression on the application tested.

--
Guillaume