Re: BUG #6061: Progresql.exe memory usage using HOLD cursor.

Lists: pgsql-bugs
From: "Yann" <yann(dot)delorme(at)esker(dot)fr>
To: pgsql-bugs(at)postgresql(dot)org
Subject: BUG #6061: Progresql.exe memory usage using HOLD cursor.
Date: 2011-06-15 13:00:40
Message-ID: 201106151300.p5FD0eH9036477@wwwmaster.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs


The following bug has been logged online:

Bug reference: 6061
Logged by: Yann
Email address: yann(dot)delorme(at)esker(dot)fr
PostgreSQL version: 9.0.4
Operating system: Windows 2008 R2
Description: Progresql.exe memory usage using HOLD cursor.
Details:

Hello,

I use POSTGRESQL 9.0.4 (64bits on windws 2008R2). The code seems to be the
same in 9.1

I execute a query with a « BINARY CURSOR WITH HOLD FOR » cursor.
The resultset contains 20.000 rows, the row size is 20 KB. I fetch result
line per line.

The issue is that in this case all rows are store in memory instead of file
in the process postgresql.exe

I think the issue is in the file tuplestore.c.

When a tuple is added the function static void
tuplestore_puttuple_common(Tuplestorestate *state, void *tuple), USEMEM is
not called with tuple size.

In my postgresql.conf, memory available is 1MB, so to reach the status
TSS_WRITEFILE, the memory tupleStore accept 256.000 rows.

In my test postgresql.exe need more than 400MB to store the resultset, in my
opinion it should use a file to store the result.

I think that, after adding the tuple in the array, a call to USEMEM should
be done.

Can you confirm that it is an issue ?

Regards.

static void
tuplestore_puttuple_common(Tuplestorestate *state, void *tuple)
{
TSReadPointer *readptr;
int i;
ResourceOwner oldowner;

switch (state->status)
{
case TSS_INMEM:

/* Stash the tuple in the
in-memory array */

state->memtuples[state->memtupcount++] = tuple;

#################################################
#################################################
########## Call USEMEM with the tuple size.
#################################################
#################################################

/*
* Done if we still fit in
available memory and have array slots.
*/
if (state->memtupcount <
state->memtupsize && !LACKMEM(state))
return;

/*
* Nope; time to switch to
tape-based operation. Make sure that
* the temp file(s) are
created in suitable temp tablespaces.
*/
PrepareTempTablespaces();

/* associate the file with
the store's resource owner */
oldowner =
CurrentResourceOwner;
CurrentResourceOwner =
state->resowner;

state->myfile =
BufFileCreateTemp(state->interXact);

CurrentResourceOwner =
oldowner;

/*
* Freeze the decision about
whether trailing length words will be
* used. We can't change this
choice once data is on tape, even
* though callers might drop
the requirement.
*/
state->backward =
(state->eflags & EXEC_FLAG_BACKWARD) != 0;
state->status =
TSS_WRITEFILE;
dumptuples(state);
break;

case TSS_WRITEFILE:

/*
* Update read pointers as
needed; see API spec above. Note:
* BufFileTell is quite cheap,
so not worth trying to avoid
* multiple calls.
*/
readptr = state->readptrs;
for (i = 0; i <
state->readptrcount; readptr++, i++)
{
if
(readptr->eof_reached && i != state->activeptr)
{

readptr->eof_reached = false;

BufFileTell(state->myfile,

&readptr->file,

&readptr->offset);
}
}

WRITETUP(state, tuple);
break;
case TSS_READFILE:

/*
* Switch from reading to
writing.
*/
if
(!state->readptrs[state->activeptr].eof_reached)

BufFileTell(state->myfile,

&state->readptrs[state->activeptr].file,


&state->readptrs[state->activeptr].offset);
if
(BufFileSeek(state->myfile,

state->writepos_file,
state->writepos_offset,

SEEK_SET) != 0)
elog(ERROR,
"tuplestore seek to EOF failed");
state->status =
TSS_WRITEFILE;

/*
* Update read pointers as
needed; see API spec above.
*/
readptr = state->readptrs;
for (i = 0; i <
state->readptrcount; readptr++, i++)
{
if
(readptr->eof_reached && i != state->activeptr)
{

readptr->eof_reached = false;

readptr->file = state->writepos_file;

readptr->offset = state->writepos_offset;
}
}

WRITETUP(state, tuple);
break;
default:
elog(ERROR, "invalid
tuplestore state");
break;
}
}

Yann.


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Yann" <yann(dot)delorme(at)esker(dot)fr>
Cc: pgsql-bugs(at)postgresql(dot)org
Subject: Re: BUG #6061: Progresql.exe memory usage using HOLD cursor.
Date: 2011-06-15 16:43:21
Message-ID: 12398.1308156201@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs

"Yann" <yann(dot)delorme(at)esker(dot)fr> writes:
> The issue is that in this case all rows are store in memory instead of file
> in the process postgresql.exe

> I think the issue is in the file tuplestore.c.
> When a tuple is added the function static void
> tuplestore_puttuple_common(Tuplestorestate *state, void *tuple), USEMEM is
> not called with tuple size.

Hmm ... yeah, I think there's a leak there.

> I think that, after adding the tuple in the array, a call to USEMEM should
> be done.

No, the callers of tuplestore_puttuple_common are supposed to do that.
But it looks like tuplestore_putvalues() forgot to do so. So data loads
that go through that particular API would miss incrementing the space
counter.

regards, tom lane


From: "Delorme, Yann" <Yann(dot)Delorme(at)esker(dot)fr>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: BUG #6061: Progresql.exe memory usage using HOLD cursor.
Date: 2011-06-16 07:37:11
Message-ID: 16940004307E8140B1AFF255B4C3B860FF9F@LY-EX10-MB-1.esker.corp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs

Thanks

Do you think that it will be fix in future release 9.1 ?

Regards,

Yann

Yann Delorme
Senior Software Engineer / Senior Software Engineer
Esker SA
Tél : +33 (0)4 72 83 46 46

Fax : + 33 (0)4 72 83 46 40
mailto:Yann(dot)Delorme(at)esker(dot)fr
http://www.esker.fr/http://www.flydoc.fr/

CONFIDENTIALITE : Ce message et les éventuelles pièces jointes sont confidentiels. Si vous n'êtes pas dans la liste des destinataires, veuillez informer l'expéditeur immédiatement et ne pas divulguer le contenu à une tierce personne. Les idées et opinions présentées dans ce message sont celles de son auteur, et ne représentent pas nécessairement celles de la société. Par ailleurs et malgré toutes les précautions prises pour éviter la présence de virus dans nos envois, nous vous recommandons de prendre, de votre côté, les mesures permettant d'assurer la non-introduction de virus dans votre système informatique. La société ne saurait être tenue pour responsable de tout dommage causé par la présence d'un virus dans ce message.
__________
-----Message d'origine-----

De : Tom Lane [mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us]
Envoyé : mercredi 15 juin 2011 18:43
À : Delorme, Yann
Cc : pgsql-bugs(at)postgresql(dot)org
Objet : Re: [BUGS] BUG #6061: Progresql.exe memory usage using HOLD cursor.

"Yann" <yann(dot)delorme(at)esker(dot)fr> writes:
> The issue is that in this case all rows are store in memory instead of
> file in the process postgresql.exe

> I think the issue is in the file tuplestore.c.
> When a tuple is added the function static void
> tuplestore_puttuple_common(Tuplestorestate *state, void *tuple),
> USEMEM is not called with tuple size.

Hmm ... yeah, I think there's a leak there.

> I think that, after adding the tuple in the array, a call to USEMEM
> should be done.

No, the callers of tuplestore_puttuple_common are supposed to do that.
But it looks like tuplestore_putvalues() forgot to do so. So data loads that go through that particular API would miss incrementing the space counter.

regards, tom lane


From: Guillaume Smet <guillaume(dot)smet(at)gmail(dot)com>
To: "Delorme, Yann" <Yann(dot)Delorme(at)esker(dot)fr>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-bugs(at)postgresql(dot)org
Subject: Re: BUG #6061: Progresql.exe memory usage using HOLD cursor.
Date: 2011-06-16 08:00:40
Message-ID: BANLkTi=vK+YorYBnh-Rfwpkxhn8NKdZ2pA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs

Yann,

2011/6/16 Delorme, Yann <Yann(dot)Delorme(at)esker(dot)fr>:
> Thanks
>
> Do you think that it will be fix in future release 9.1 ?

Tom commited a fix in all the supported releases where the bug is
present including 9.0:
http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=669ac03af62328e4eb572dacb8ba319414ef1211

You can either build a specific 9.0 release with the patch or wait for
the next 9.0.x maintenance release.

Have a nice day.

--
Guillaume