Обсуждение: aggregation memory leak and fix
Platform: Alpha, Digital UNIX 4.0D
Software: PostgreSQL 6.4.2 and 6.5 snaphot (11 March 1999)
I have a table as follows:
Table = lineitem
+------------------------+----------------------------------+-------+
| Field | Type | Length|
+------------------------+----------------------------------+-------+
| l_orderkey | int4 not null | 4 |
| l_partkey | int4 not null | 4 |
| l_suppkey | int4 not null | 4 |
| l_linenumber | int4 not null | 4 |
| l_quantity | float4 not null | 4 |
| l_extendedprice | float4 not null | 4 |
| l_discount | float4 not null | 4 |
| l_tax | float4 not null | 4 |
| l_returnflag | char() not null | 1 |
| l_linestatus | char() not null | 1 |
| l_shipdate | date | 4 |
| l_commitdate | date | 4 |
| l_receiptdate | date | 4 |
| l_shipinstruct | char() not null | 25 |
| l_shipmode | char() not null | 10 |
| l_comment | char() not null | 44 |
+------------------------+----------------------------------+-------+
Index: lineitem_index_
that ends up having on the order of 500,000 rows (about 100 MB on disk).
I then run an aggregation query as:
--
-- Query 1
--
select l_returnflag, l_linestatus, sum(l_quantity) as sum_qty,
sum(l_extendedprice) as sum_base_price,
sum(l_extendedprice*(1-l_discount)) as sum_disc_price,
sum(l_extendedprice*(1-l_discount)*(1+l_tax)) as sum_charge,
avg(l_quantity) as avg_qty, avg(l_extendedprice) as avg_price,
avg(l_discount) as avg_disc, count(*) as count_order
from lineitem
where l_shipdate <= ('1998-12-01'::datetime - interval '90 day')::date
group by l_returnflag, l_linestatus
order by l_returnflag, l_linestatus;
when I run this against 6.4.2, the postgres process grows to upwards of
1 GB of memory (at which point something overflows and it dumps core) -
I watch it grow through 200 MB, 400 MB, 800 MB, dies somewhere near 1 GB
of allocated memory).
If I take out a few of the "sum" expressions it gets better, removing
sum_disk_price and sum_charge causes it to be only 600 MB and the query
actually (eventually) completes. Takes about 10 minutes on my 500 MHz
machine with 256 MB core and 4 GB of swap.
The problem seems to be the memory allocation mechanism. Looking at a
call trace, it is doing some kind of "sub query" plan for each row in
the database. That means it does ExecEval and postquel_function and
postquel_execute and all their friends for each row in the database.
Allocating a couple hundred bytes for each one.
The problem is that none of these allocations are freed - they seem to
depend on the AllocSet to free them at the end of the transaction. This
means it isn't a "true" leak, because the bytes are all freed at the
(very) end of the transaction, but it does mean that the process grows
to unreasonable size in the meantime. There is no need for this,
because the individual expression results are aggregated as it goes
along, so the intermediate nodes can be freed.
I spent half a day last week chasing down the offending palloc() calls
and execution stacks sufficiently that I think I found the right places
to put pfree() calls.
As a result, I have changes in the files:
src/backend/executor/execUtils.c
src/backend/executor/nodeResult.c
src/backend/executor/nodeAgg.c
src/backend/executor/execMain.c
patches to these files are attached at the end of this message. These
files are based on the 6.5.0 snapshot downloaded from ftp.postgreql.org
on 11 March 1999.
Apologies for sending patches to a non-released version. If anyone has
problems applying the patches, I can send the full files (I wanted to
avoid sending a 100K shell archive to the list). If anyone cares about
reproducing my exact problem with the above table, I can provide the 100
MB pg_dump file for download as well.
Secondary Issue: the reason I did not use the 6.4.2 code to make my
changes is because the AllocSet calls in that one were particularly
egregious - they only had the skeleton of the allocsets code that exists
in the 6.5 snapshots, so they were calling malloc() for all of the 8 and
16 byte allocations that the above query causes.
Using the fixed code reduces the maximum memory requirement on the above
query to about 210 MB, and reduces the runtime to (an acceptable) 1.5
minutes - a factor of more than 6x improvement on my 256 MB machine.
Now the biggest part of the execution time is in the sort before the
aggregation (which isn't strictly needed, but that is an optimization
for another day).
Open Issue: there is still a small "leak" that I couldn't eliminate, I
think I chased it down to the constvalue allocated in
execQual::ExecTargetList(), but I couldn't figure out where to properly
free it. 8 bytes leaked was much better than 750 bytes, so I stopped
banging my head on that particular item.
Secondary Open Issue: what I did have to do to get down to 210 MB of
core was reduce the minimum allocation size in AllocSet to 8 bytes from
16 bytes. That reduces the 8 byte leak above to a true 8 byte, rather
than a 16 byte leak. Otherwise, I think the size was 280 MB (still a
big improvement on 1000+ MB). I only changed this in my code and I am
not including a changed mcxt.c for that.
I hope my changes are understandable/reasonable. Enjoy.
Erik Riedel
Carnegie Mellon University
www.cs.cmu.edu/~riedel
--------------[aggregation_memory_patch.sh]-----------------------
#! /bin/sh
# This is a shell archive, meaning:
# 1. Remove everything above the #! /bin/sh line.
# 2. Save the resulting text in a file.
# 3. Execute the file with /bin/sh (not csh) to create:
# execMain.c.diff
# execUtils.c.diff
# nodeAgg.c.diff
# nodeResult.c.diff
# This archive created: Fri Mar 19 15:47:17 1999
export PATH; PATH=/bin:/usr/bin:$PATH
if test -f 'execMain.c.diff'
thenecho shar: "will not over-write existing file 'execMain.c.diff'"
else
cat << \SHAR_EOF > 'execMain.c.diff'
583c
.
398a
.
396a/* XXX - clean up some more from ExecutorStart() - er1p */if (NULL == estate->es_snapshot) { /* nothing to free
*/}else { if (estate->es_snapshot->xcnt > 0) { pfree(estate->es_snapshot->xip); } pfree(estate->es_snapshot);}
if (NULL == estate->es_param_exec_vals) { /* nothing to free */} else { pfree(estate->es_param_exec_vals);
estate->es_param_exec_vals= NULL;}
.
SHAR_EOF
fi
if test -f 'execUtils.c.diff'
thenecho shar: "will not over-write existing file 'execUtils.c.diff'"
else
cat << \SHAR_EOF > 'execUtils.c.diff'
368a
}
/* ----------------* ExecFreeExprContext* ----------------*/
void
ExecFreeExprContext(CommonState *commonstate)
{ExprContext *econtext;
/* ---------------- * get expression context. if NULL then this node has * none so we just return. *
----------------*/econtext = commonstate->cs_ExprContext;if (econtext == NULL) return;
/* ---------------- * clean up memory used. * ---------------- */pfree(econtext);commonstate->cs_ExprContext =
NULL;
}
/* ----------------* ExecFreeTypeInfo* ----------------*/
void
ExecFreeTypeInfo(CommonState *commonstate)
{TupleDesc tupDesc;
tupDesc = commonstate->cs_ResultTupleSlot->ttc_tupleDescriptor;if (tupDesc == NULL) return;
/* ---------------- * clean up memory used. * ----------------
*/FreeTupleDesc(tupDesc);commonstate->cs_ResultTupleSlot->ttc_tupleDescriptor= NULL;
.
274a
.
SHAR_EOF
fi
if test -f 'nodeAgg.c.diff'
thenecho shar: "will not over-write existing file 'nodeAgg.c.diff'"
else
cat << \SHAR_EOF > 'nodeAgg.c.diff'
376a pfree(oldVal); /* XXX - new, let's free the old datum - er1p */
.
374a oldVal = value1[aggno]; /* XXX - save so we can free later - er1p */
.
112aDatum oldVal = (Datum) NULL; /* XXX - so that we can save and free on
each iteration - er1p */
.
SHAR_EOF
fi
if test -f 'nodeResult.c.diff'
thenecho shar: "will not over-write existing file 'nodeResult.c.diff'"
else
cat << \SHAR_EOF > 'nodeResult.c.diff'
278apfree(resstate); node->resstate = NULL; /* XXX - new for us - er1p */
.
265aExecFreeExprContext(&resstate->cstate); /* XXX - new for us - er1p */ExecFreeTypeInfo(&resstate->cstate); /* XXX -
newfor us - er1p */
.
SHAR_EOF
fi
exit 0
# End of shell archive
> when I run this against 6.4.2, the postgres process grows to upwards of > 1 GB of memory (at which point something overflows and it dumps core) - > I watch it grow through 200 MB, 400 MB, 800 MB, dies somewhere near 1 GB > of allocated memory). > > If I take out a few of the "sum" expressions it gets better, removing > sum_disk_price and sum_charge causes it to be only 600 MB and the query > actually (eventually) completes. Takes about 10 minutes on my 500 MHz > machine with 256 MB core and 4 GB of swap. Wow, that is large. > The problem seems to be the memory allocation mechanism. Looking at a > call trace, it is doing some kind of "sub query" plan for each row in > the database. That means it does ExecEval and postquel_function and > postquel_execute and all their friends for each row in the database. > Allocating a couple hundred bytes for each one. I will admit we really haven't looked at executor issues recently. We have had few bug reports in that area, so we normally have just left it alone. I was aware of some memory allocation issues with aggregates, but I thought I fixed them in 6.5. Obviously not. > The problem is that none of these allocations are freed - they seem to > depend on the AllocSet to free them at the end of the transaction. This > means it isn't a "true" leak, because the bytes are all freed at the > (very) end of the transaction, but it does mean that the process grows > to unreasonable size in the meantime. There is no need for this, > because the individual expression results are aggregated as it goes > along, so the intermediate nodes can be freed. Yes, but a terrible over-allocation. > I spent half a day last week chasing down the offending palloc() calls > and execution stacks sufficiently that I think I found the right places > to put pfree() calls. > > As a result, I have changes in the files: > > src/backend/executor/execUtils.c > src/backend/executor/nodeResult.c > src/backend/executor/nodeAgg.c > src/backend/executor/execMain.c > > patches to these files are attached at the end of this message. These > files are based on the 6.5.0 snapshot downloaded from ftp.postgreql.org > on 11 March 1999. > > Apologies for sending patches to a non-released version. If anyone has > problems applying the patches, I can send the full files (I wanted to > avoid sending a 100K shell archive to the list). If anyone cares about > reproducing my exact problem with the above table, I can provide the 100 > MB pg_dump file for download as well. No apologies necessary. Glad to have someone digging into that area of the code. We will gladly apply your patches to 6.5. However, I request that you send context diffs(diff -c). Normal diffs are just too error-prone in application. Send them, and I will apply them right away. > Secondary Issue: the reason I did not use the 6.4.2 code to make my > changes is because the AllocSet calls in that one were particularly > egregious - they only had the skeleton of the allocsets code that exists > in the 6.5 snapshots, so they were calling malloc() for all of the 8 and > 16 byte allocations that the above query causes. Glad you used 6.5. Makes it easier to merge them into our next release. > Using the fixed code reduces the maximum memory requirement on the above > query to about 210 MB, and reduces the runtime to (an acceptable) 1.5 > minutes - a factor of more than 6x improvement on my 256 MB machine. > > Now the biggest part of the execution time is in the sort before the > aggregation (which isn't strictly needed, but that is an optimization > for another day). Not sure why that is there? Perhaps for GROUP BY processing? > > Open Issue: there is still a small "leak" that I couldn't eliminate, I > think I chased it down to the constvalue allocated in > execQual::ExecTargetList(), but I couldn't figure out where to properly > free it. 8 bytes leaked was much better than 750 bytes, so I stopped > banging my head on that particular item. Can you give me the exact line? Is it the palloc(1)? > Secondary Open Issue: what I did have to do to get down to 210 MB of > core was reduce the minimum allocation size in AllocSet to 8 bytes from > 16 bytes. That reduces the 8 byte leak above to a true 8 byte, rather > than a 16 byte leak. Otherwise, I think the size was 280 MB (still a > big improvement on 1000+ MB). I only changed this in my code and I am > not including a changed mcxt.c for that. Maybe Jan, our memory optimizer, can discuss the 8 vs. 16 byte issue. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> No apologies necessary. Glad to have someone digging into that area of
> the code. We will gladly apply your patches to 6.5. However, I request
> that you send context diffs(diff -c). Normal diffs are just too
> error-prone in application. Send them, and I will apply them right
> away.
>
Context diffs attached. This was due to my ignorance of diff. When I
made the other files, I though "hmm, these could be difficult to apply
if the code has changed a bit, wouldn't it be good if they included a
few lines before and after the fix". Now I know "-c".
> Not sure why that is there? Perhaps for GROUP BY processing?
>
Right, it is a result of the Group processing requiring sorted input.
Just that it doesn't "require" sorted input, it "could" be a little more
flexible and the sort wouldn't be necessary. Essentially this would be
a single "AggSort" node that did the aggregation while sorting (probably
with replacement selection rather than quicksort). This definitely
would require some code/smarts that isn't there today.
> > think I chased it down to the constvalue allocated in
> > execQual::ExecTargetList(), but I couldn't figure out where to properly
> > free it. 8 bytes leaked was much better than 750 bytes, so I stopped
> > banging my head on that particular item.
>
> Can you give me the exact line? Is it the palloc(1)?
>
No, the 8 bytes seem to come from the ExecEvalExpr() call near line
1530. Problem was when I tried to free these, I got "not in AllocSet"
errors, so something more complicated was going on.
Thanks.
Erik
-----------[aggregation_memory_patch.sh]----------------------
#! /bin/sh
# This is a shell archive, meaning:
# 1. Remove everything above the #! /bin/sh line.
# 2. Save the resulting text in a file.
# 3. Execute the file with /bin/sh (not csh) to create:
# execMain.c.diff
# execUtils.c.diff
# nodeAgg.c.diff
# nodeResult.c.diff
# This archive created: Fri Mar 19 19:35:42 1999
export PATH; PATH=/bin:/usr/bin:$PATH
if test -f 'execMain.c.diff'
thenecho shar: "will not over-write existing file 'execMain.c.diff'"
else
cat << \SHAR_EOF > 'execMain.c.diff'
***
/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/611/src/backend/executor/
execMain.c Thu Mar 11 23:59:11 1999
---
/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/612/src/backend/executor/
execMain.c Fri Mar 19 15:03:28 1999
***************
*** 394,401 ****
--- 394,419 ---- EndPlan(queryDesc->plantree, estate);
+ /* XXX - clean up some more from ExecutorStart() - er1p */
+ if (NULL == estate->es_snapshot) {
+ /* nothing to free */
+ } else {
+ if (estate->es_snapshot->xcnt > 0) {
+ pfree(estate->es_snapshot->xip);
+ }
+ pfree(estate->es_snapshot);
+ }
+
+ if (NULL == estate->es_param_exec_vals) {
+ /* nothing to free */
+ } else {
+ pfree(estate->es_param_exec_vals);
+ estate->es_param_exec_vals = NULL;
+ }
+ /* restore saved refcounts. */ BufferRefCountRestore(estate->es_refcount);
+ } void
***************
*** 580,586 **** /* * initialize result relation stuff */
! if (resultRelation != 0 && operation != CMD_SELECT) { /*
--- 598,604 ---- /* * initialize result relation stuff */
! if (resultRelation != 0 && operation != CMD_SELECT) { /*
SHAR_EOF
fi
if test -f 'execUtils.c.diff'
thenecho shar: "will not over-write existing file 'execUtils.c.diff'"
else
cat << \SHAR_EOF > 'execUtils.c.diff'
***
/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/611/src/backend/executor/
execUtils.c Thu Mar 11 23:59:11 1999
---
/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/612/src/backend/executor/
execUtils.c Fri Mar 19 14:55:59 1999
***************
*** 272,277 ****
--- 272,278 ---- #endif i++; }
+ if (len > 0) { ExecAssignResultType(commonstate,
***************
*** 366,371 ****
--- 367,419 ---- pfree(projInfo); commonstate->cs_ProjInfo = NULL;
+ }
+
+ /* ----------------
+ * ExecFreeExprContext
+ * ----------------
+ */
+ void
+ ExecFreeExprContext(CommonState *commonstate)
+ {
+ ExprContext *econtext;
+
+ /* ----------------
+ * get expression context. if NULL then this node has
+ * none so we just return.
+ * ----------------
+ */
+ econtext = commonstate->cs_ExprContext;
+ if (econtext == NULL)
+ return;
+
+ /* ----------------
+ * clean up memory used.
+ * ----------------
+ */
+ pfree(econtext);
+ commonstate->cs_ExprContext = NULL;
+ }
+
+ /* ----------------
+ * ExecFreeTypeInfo
+ * ----------------
+ */
+ void
+ ExecFreeTypeInfo(CommonState *commonstate)
+ {
+ TupleDesc tupDesc;
+
+ tupDesc = commonstate->cs_ResultTupleSlot->ttc_tupleDescriptor;
+ if (tupDesc == NULL)
+ return;
+
+ /* ----------------
+ * clean up memory used.
+ * ----------------
+ */
+ FreeTupleDesc(tupDesc);
+ commonstate->cs_ResultTupleSlot->ttc_tupleDescriptor = NULL; } /*
----------------------------------------------------------------
SHAR_EOF
fi
if test -f 'nodeAgg.c.diff'
thenecho shar: "will not over-write existing file 'nodeAgg.c.diff'"
else
cat << \SHAR_EOF > 'nodeAgg.c.diff'
***
/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/611/src/backend/executor/
nodeAgg.c Thu Mar 11 23:59:11 1999
---
/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/612/src/backend/executor/
nodeAgg.c Fri Mar 19 15:01:21 1999
***************
*** 110,115 ****
--- 110,116 ---- isNull2 = FALSE; bool qual_result;
+ Datum oldVal = (Datum) NULL; /* XXX - so that we can save and free
on each iteration - er1p */ /* --------------------- * get state info from node
***************
*** 372,379 ****
--- 373,382 ---- */ args[0] = value1[aggno];
args[1]= newVal;
+ oldVal = value1[aggno]; /* XXX - save so we can free later - er1p */
value1[aggno]= (Datum) fmgr_c(&aggfns->xfn1, (FmgrValues *) args,
&isNull1);
+ pfree(oldVal); /* XXX - new, let's free the old datum - er1p */
Assert(!isNull1); } }
SHAR_EOF
fi
if test -f 'nodeResult.c.diff'
thenecho shar: "will not over-write existing file 'nodeResult.c.diff'"
else
cat << \SHAR_EOF > 'nodeResult.c.diff'
***
/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/611/src/backend/executor/
nodeResult.c Thu Mar 11 23:59:12 1999
---
/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/612/src/backend/executor/
nodeResult.c Fri Mar 19 14:57:26 1999
***************
*** 263,268 ****
--- 263,270 ---- * is freed at end-transaction time. -cim 6/2/91 * ---------------- */
+ ExecFreeExprContext(&resstate->cstate); /* XXX - new for us - er1p */
+ ExecFreeTypeInfo(&resstate->cstate); /* XXX - new for us - er1p */ ExecFreeProjectionInfo(&resstate->cstate);
/* ----------------
***************
*** 276,281 ****
--- 278,284 ---- * ---------------- */ ExecClearTuple(resstate->cstate.cs_ResultTupleSlot);
+ pfree(resstate); node->resstate = NULL; /* XXX - new for us - er1p */ } void
SHAR_EOF
fi
exit 0
# End of shell archive
> > > No apologies necessary. Glad to have someone digging into that area of > > the code. We will gladly apply your patches to 6.5. However, I request > > that you send context diffs(diff -c). Normal diffs are just too > > error-prone in application. Send them, and I will apply them right > > away. > > > Context diffs attached. This was due to my ignorance of diff. When I > made the other files, I though "hmm, these could be difficult to apply > if the code has changed a bit, wouldn't it be good if they included a > few lines before and after the fix". Now I know "-c". Applied. > > Not sure why that is there? Perhaps for GROUP BY processing? > > > Right, it is a result of the Group processing requiring sorted input. > Just that it doesn't "require" sorted input, it "could" be a little more > flexible and the sort wouldn't be necessary. Essentially this would be > a single "AggSort" node that did the aggregation while sorting (probably > with replacement selection rather than quicksort). This definitely > would require some code/smarts that isn't there today. I think you will find make_groupPlan adds the sort as needed by the GROUP BY. I assume you are suggesting to do the aggregate/GROUP on unsorted data, which is hard to do in a flexible way. > > > think I chased it down to the constvalue allocated in > > > execQual::ExecTargetList(), but I couldn't figure out where to properly > > > free it. 8 bytes leaked was much better than 750 bytes, so I stopped > > > banging my head on that particular item. > > > > Can you give me the exact line? Is it the palloc(1)? > > > No, the 8 bytes seem to come from the ExecEvalExpr() call near line > 1530. Problem was when I tried to free these, I got "not in AllocSet" > errors, so something more complicated was going on. Yes, if you look inside ExecEvalExpr(), you will see it tries to get a value for the expression(Datum). It may return an int, float4, or a string. In the last case, that is actually a pointer and not a specific value. So, in some cases, the value can just be thrown away, or it may be a pointer to memory that can be freed after the call to heap_formtuple() later in the function. The trick is to find the function call in ExecEvalExpr() that is allocating something, and conditionally free values[] after the call to heap_formtuple(). If you don't want find it, perhaps you can send me enough info so I can see it here. I wonder whether it is the call to CreateTupleDescCopy() inside ExecEvalVar()? Another problem I just fixed is that fjIsNull was not being pfree'ed if it was used with >64 targets, but I don't think that affects you. I also assume you have run your recent patch through the the test/regression tests, so see it does not cause some other area to fail, right? -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> Yes, if you look inside ExecEvalExpr(), you will see it tries to get a > value for the expression(Datum). It may return an int, float4, or a > string. In the last case, that is actually a pointer and not a specific > value. > > So, in some cases, the value can just be thrown away, or it may be a > pointer to memory that can be freed after the call to heap_formtuple() > later in the function. The trick is to find the function call in > ExecEvalExpr() that is allocating something, and conditionally free > values[] after the call to heap_formtuple(). If you don't want find it, > perhaps you can send me enough info so I can see it here. > > I wonder whether it is the call to CreateTupleDescCopy() inside > ExecEvalVar()? I am now not totally sure about what I said above. The general plan, though is accurate, that perhaps something is being allocated. I need to see the query again, which I don't have anymore. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
>
> Platform: Alpha, Digital UNIX 4.0D
> Software: PostgreSQL 6.4.2 and 6.5 snaphot (11 March 1999)
>
> I have a table as follows:
>
> Table = lineitem
> +------------------------+----------------------------------+-------+
> | Field | Type | Length|
> +------------------------+----------------------------------+-------+
> | l_orderkey | int4 not null | 4 |
> | l_partkey | int4 not null | 4 |
> | l_suppkey | int4 not null | 4 |
> | l_linenumber | int4 not null | 4 |
> | l_quantity | float4 not null | 4 |
> | l_extendedprice | float4 not null | 4 |
> | l_discount | float4 not null | 4 |
> | l_tax | float4 not null | 4 |
> | l_returnflag | char() not null | 1 |
> | l_linestatus | char() not null | 1 |
> | l_shipdate | date | 4 |
> | l_commitdate | date | 4 |
> | l_receiptdate | date | 4 |
> | l_shipinstruct | char() not null | 25 |
> | l_shipmode | char() not null | 10 |
> | l_comment | char() not null | 44 |
> +------------------------+----------------------------------+-------+
> Index: lineitem_index_
>
> that ends up having on the order of 500,000 rows (about 100 MB on disk).
>
> I then run an aggregation query as:
>
> --
> -- Query 1
> --
> select l_returnflag, l_linestatus, sum(l_quantity) as sum_qty,
> sum(l_extendedprice) as sum_base_price,
> sum(l_extendedprice*(1-l_discount)) as sum_disc_price,
> sum(l_extendedprice*(1-l_discount)*(1+l_tax)) as sum_charge,
> avg(l_quantity) as avg_qty, avg(l_extendedprice) as avg_price,
> avg(l_discount) as avg_disc, count(*) as count_order
> from lineitem
> where l_shipdate <= ('1998-12-01'::datetime - interval '90 day')::date
> group by l_returnflag, l_linestatus
> order by l_returnflag, l_linestatus;
>
OK, I do have the query. Please try removing the (1+l_tax) so it is
just l_tax, and change the 1998... to just a simple date string, and see
if the problem goes away. If we can find something specific in the
query that is causing the memory over-allocation, it is that much easier
to find the cause.
Also, try removing all the arithmetic in the query or simplify the query
to see if there is a certain part that is causing it. If it is really
an 8-byte issue, it must be very small indeed, and only visible because
you have so much data, and are attentive.
-- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610)
853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill,
Pennsylvania19026
> select l_returnflag, l_linestatus, sum(l_quantity) as sum_qty,
> sum(l_extendedprice) as sum_base_price,
> sum(l_extendedprice*(1-l_discount)) as sum_disc_price,
> sum(l_extendedprice*(1-l_discount)*(1+l_tax)) as sum_charge,
> avg(l_quantity) as avg_qty, avg(l_extendedprice) as avg_price,
> avg(l_discount) as avg_disc, count(*) as count_order
> from lineitem
> where l_shipdate <= ('1998-12-01'::datetime - interval '90 day')::date
> group by l_returnflag, l_linestatus
> order by l_returnflag, l_linestatus;
OK, I have researched this. I think you will find that:
('1998-12-01'::datetime - interval '90 day')::date
is the cause. In datetime_mi(), you will see: dt1 = *datetime1; dt2 = *datetime2; result =
palloc(sizeof(TimeSpan)); ^^^^^^ if (DATETIME_IS_RELATIVE(dt1)) dt1 = SetDateTime(dt1);
if(DATETIME_IS_RELATIVE(dt2)) dt2 = SetDateTime(dt2);
This obviously shows us allocating a return value, that probably is not
free'ed until the end of the query. TimeSpan is: typedef struct{ double time; /* all time units
otherthan months and * years */ int4 month; /* months and years,
aftertime for * alignment */} TimeSpan;
Now, we certainly could pre-compute this constant once before doing the
query, but we don't, and even if we did, this would not fix the case
where a Var is involved in the expression.
When we grab values directly from tuples, like Var, the tuples are
auto-free'ed at the end, because they exist in tuple that we track.
Values computed inside very deep functions are tough for us to free. In
fact, this could be a very complex expression, with a variety of
temporary palloc'ed values used during the process.
My only quick solution would seem to be to add a new "expression" memory
context, that can be cleared after every tuple is processed, clearing
out temporary values allocated inside an expression. This probably
could be done very easily, because the entry/exit locations into the
expression system are very limited.
Ideas people?
-- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610)
853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill,
Pennsylvania19026
>
> > No apologies necessary. Glad to have someone digging into that area of
> > the code. We will gladly apply your patches to 6.5. However, I request
> > that you send context diffs(diff -c). Normal diffs are just too
> > error-prone in application. Send them, and I will apply them right
> > away.
> >
> Context diffs attached. This was due to my ignorance of diff. When I
> made the other files, I though "hmm, these could be difficult to apply
> if the code has changed a bit, wouldn't it be good if they included a
> few lines before and after the fix". Now I know "-c".
We are seeing regression failure on aggregates after the patches. It is
happening in nodeAgg.c, line 379:
pfree(oldVal); /* XXX - new, let's free the old datum -$
-- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610)
853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill,
Pennsylvania19026
> > > No apologies necessary. Glad to have someone digging into that area of > > the code. We will gladly apply your patches to 6.5. However, I request > > that you send context diffs(diff -c). Normal diffs are just too > > error-prone in application. Send them, and I will apply them right > > away. > > > Context diffs attached. This was due to my ignorance of diff. When I > made the other files, I though "hmm, these could be difficult to apply > if the code has changed a bit, wouldn't it be good if they included a > few lines before and after the fix". Now I know "-c". I have had to back out the nodeAgg.c part of the patch. The rest looks OK, partly because it is dealing with freeing expression context, and partly because I don't understand it all. The nodeAgg.c part of the patch is clearly trying to free memory allocated as intermediate parts of the expression, and has to be solved by a more general solution as I discussed. If you look in backend/utils/adt/*.c, you will see lots of intermediate memory allocated. The allocacations as part of the *in/*out functions are not a problem, because they are called only in other areas, and are freed, but the other ones are probably not free'ed until statement termination. I may need to specifically put those pfree's in their own context, and free them on tuple completion. I am waiting to hear what others say about my ideas. Feel free to keep digging. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <maillist@candle.pha.pa.us> writes:
> My only quick solution would seem to be to add a new "expression" memory
> context, that can be cleared after every tuple is processed, clearing
> out temporary values allocated inside an expression.
Right, this whole problem of growing backend memory use during a large
SELECT (or COPY, or probably a few other things) is one of the things
that we were talking about addressing by revising the memory management
structure.
I think what we want inside the executor is a distinction between
storage that must live to the end of the statement and storage that is
only needed while processing the current tuple. The second kind of
storage would go into a separate context that gets flushed every so
often. (It could be every tuple, or every dozen or hundred tuples
depending on what seems the best tradeoff of cycles against memory
usage.)
I'm not sure that just two contexts is enough, either. For example inSELECT field1, SUM(field2) GROUP BY field1;
the working memory for the SUM aggregate could not be released after
each tuple, but perhaps we don't want it to live for the whole statement
either --- in that case we'd need a per-group context. (This particular
example isn't very convincing, because the same storage for the SUM
*could* be recycled from group to group. But I don't know whether it
actually *is* reused or not. If fresh storage is palloc'd for each
instantiation of SUM then we have a per-group leak in this scenario.
In any case, I'm not sure all aggregate functions have constant memory
requirements that would let them recycle storage across groups.)
What we need to do is work out what the best set of memory context
definitions is, and then decide on a strategy for making sure that
lower-level routines allocate their return values in the right context.
It'd be nice if the lower-level routines could still call palloc() and
not have to worry about this explicitly --- otherwise we'll break not
only a lot of our own code but perhaps a lot of user code. (User-
specific data types and SPI code all use palloc, no?)
I think it is too late to try to fix this for 6.5, but it ought to be a
top priority for 6.6.
regards, tom lane
> Bruce Momjian <maillist@candle.pha.pa.us> writes:
> > My only quick solution would seem to be to add a new "expression" memory
> > context, that can be cleared after every tuple is processed, clearing
> > out temporary values allocated inside an expression.
>
> Right, this whole problem of growing backend memory use during a large
> SELECT (or COPY, or probably a few other things) is one of the things
> that we were talking about addressing by revising the memory management
> structure.
>
> I think what we want inside the executor is a distinction between
> storage that must live to the end of the statement and storage that is
> only needed while processing the current tuple. The second kind of
> storage would go into a separate context that gets flushed every so
> often. (It could be every tuple, or every dozen or hundred tuples
> depending on what seems the best tradeoff of cycles against memory
> usage.)
>
> I'm not sure that just two contexts is enough, either. For example in
> SELECT field1, SUM(field2) GROUP BY field1;
> the working memory for the SUM aggregate could not be released after
> each tuple, but perhaps we don't want it to live for the whole statement
> either --- in that case we'd need a per-group context. (This particular
> example isn't very convincing, because the same storage for the SUM
> *could* be recycled from group to group. But I don't know whether it
> actually *is* reused or not. If fresh storage is palloc'd for each
> instantiation of SUM then we have a per-group leak in this scenario.
> In any case, I'm not sure all aggregate functions have constant memory
> requirements that would let them recycle storage across groups.)
>
> What we need to do is work out what the best set of memory context
> definitions is, and then decide on a strategy for making sure that
> lower-level routines allocate their return values in the right context.
> It'd be nice if the lower-level routines could still call palloc() and
> not have to worry about this explicitly --- otherwise we'll break not
> only a lot of our own code but perhaps a lot of user code. (User-
> specific data types and SPI code all use palloc, no?)
Let me make an argument here.
Let's suppose that we want to free all the memory used as expression
intermediate values after each row is processed.
It is my understanding that all these are created in utils/adt/*.c
files, and that the entry point to all those functions via
fmgr()/fmgr_c().
So, if we go into an expression memory context before calling
fmgr/fmgr_c in the executor, and return to the normal context after the
function call, all our intermediates are trapped in the expression
memory context.
At the end of each row, we just free the expression memory context. In
almost all cases, the data is stored in tuples, and we can free it. In
a few cases like aggregates, we have to save off the value we need to
keep before freeing the expression context. In fact, you could even
optimize the cleanup to only do free'ing if some expression memory was
allocated. In most cases, it is not.
In fact the nodeAgg.c patch that I backed out attempted to do that,
though because there wasn't code that checked if the Datum was
pg_type.typbyval, it didn't work 100%.
In fact, a quick look at the executor shows:#$ grep "fmgr(" *.c |detab -t 4execUtils.c: predString = fmgr(F_TEXTOUT,
&indexStruct->indpred);nodeGroup.c: val1 = fmgr(typoutput, attr1, typelem,nodeGroup.c: val2 = fmgr(typoutput,
attr2,typelem,nodeUnique.c: val1 = fmgr(typoutput, attr1, typelem,nodeUnique.c: val2 = fmgr(typoutput, attr2,
typelem,spi.c: return (fmgr(foutoid, val, typelem,
#$ grep "fmgr_c(" *.c |detab -t 4execQual.c: return (Datum) fmgr_c(&fcache->func, (FmgrValues *)argV,
isNull);nodeAgg.c: value1[aggno] = (Datum) fmgr_c(&aggfns->xfn1,nodeAgg.c: value2[aggno] = (Datum)
fmgr_c(&aggfns->xfn2,nodeAgg.c: value1[aggno] = (Datum) fmgr_c(&aggfns->finalfn,
The fmgr(out*) calls are probably not an issue, because they are already
cleaned up. The only issue are the fmgr_c calls. execQual is the MAJOR
one for all expressions, and the nodeAgg calls would have to have some
saving of the last entry done.
Seems pretty straight-forward to me. The fact is we have a pretty clean
memory allocation system. Not sure if a redesign is necessary if we can
clean up any per-tuple allocations, and I think this would do the trick.
Comments?
-- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610)
853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill,
Pennsylvania19026
Bruce Momjian <maillist@candle.pha.pa.us> writes:
>> What we need to do is work out what the best set of memory context
>> definitions is, and then decide on a strategy for making sure that
>> lower-level routines allocate their return values in the right context.
> Let's suppose that we want to free all the memory used as expression
> intermediate values after each row is processed.
> It is my understanding that all these are created in utils/adt/*.c
> files, and that the entry point to all those functions via
> fmgr()/fmgr_c().
That's probably the bulk of the specific calls of palloc(). Someone
(Jan?) did a scan of the code a while ago looking for palloc() calls,
and there aren't that many outside of the data-type-specific functions.
But we'd have to look individually at all the ones that are elsewhere.
> So, if we go into an expression memory context before calling
> fmgr/fmgr_c in the executor, and return to the normal context after the
> function call, all our intermediates are trapped in the expression
> memory context.
OK, so you're saying we leave the data-type-specific functions as is
(calling palloc() to allocate their result areas), and make each call
site specifically responsible for setting the context that palloc() will
allocate from? That could work, I think. We'd need to see what side
effects it'd have on other uses of palloc().
What we'd probably want is to use a stack discipline for the current
palloc-target memory context: when you set the context, you get back the
ID of the old context, and you are supposed to restore that old context
before returning.
> At the end of each row, we just free the expression memory context. In
> almost all cases, the data is stored in tuples, and we can free it. In
> a few cases like aggregates, we have to save off the value we need to
> keep before freeing the expression context.
Actually, nodeAgg would just have to set an appropriate context before
calling fmgr to execute the aggregate's transition functions, and then
it wouldn't need an extra copy step. The results would come back in the
right context already.
> In fact, you could even optimize the cleanup to only do free'ing if
> some expression memory was allocated. In most cases, it is not.
Jan's stuff should already fall through pretty quickly if there's
nothing in the context, I think. Note that what we want to do between
tuples is a "context clear" of the expression context, not a "context
delete" and then "context create" a new expression context. Context
clear should be a pretty quick no-op if nothing's been allocated in that
context...
> In fact the nodeAgg.c patch that I backed out attempted to do that,
> though because there wasn't code that checked if the Datum was
> pg_type.typbyval, it didn't work 100%.
Right. But if we approach it this way (clear the context at appropriate
times) rather than thinking in terms of explicitly pfree'ing individual
objects, life gets much simpler. Also, if we insist on being able to
pfree individual objects inside a context, we can't use Jan's faster
allocator! Remember, the reason it is faster and lower overhead is that
it doesn't keep track of individual objects, only pools.
I'd like to see us head in the direction of removing most of the
explicit pfree calls that exist now, and instead rely on clearing
memory contexts at appropriate times in order to manage memory.
The fewer places where we need pfree, the more contexts can be run
with the low-overhead space allocator. Also, the fewer explicit
pfrees we need, the simpler and more reliable the code gets.
regards, tom lane
> Bruce Momjian <maillist@candle.pha.pa.us> writes: > >> What we need to do is work out what the best set of memory context > >> definitions is, and then decide on a strategy for making sure that > >> lower-level routines allocate their return values in the right context. > > > Let's suppose that we want to free all the memory used as expression > > intermediate values after each row is processed. > > It is my understanding that all these are created in utils/adt/*.c > > files, and that the entry point to all those functions via > > fmgr()/fmgr_c(). > > That's probably the bulk of the specific calls of palloc(). Someone > (Jan?) did a scan of the code a while ago looking for palloc() calls, > and there aren't that many outside of the data-type-specific functions. > But we'd have to look individually at all the ones that are elsewhere. > > > So, if we go into an expression memory context before calling > > fmgr/fmgr_c in the executor, and return to the normal context after the > > function call, all our intermediates are trapped in the expression > > memory context. > > OK, so you're saying we leave the data-type-specific functions as is > (calling palloc() to allocate their result areas), and make each call > site specifically responsible for setting the context that palloc() will > allocate from? That could work, I think. We'd need to see what side > effects it'd have on other uses of palloc(). > > What we'd probably want is to use a stack discipline for the current > palloc-target memory context: when you set the context, you get back the > ID of the old context, and you are supposed to restore that old context > before returning. > > > At the end of each row, we just free the expression memory context. In > > almost all cases, the data is stored in tuples, and we can free it. In > > a few cases like aggregates, we have to save off the value we need to > > keep before freeing the expression context. > > Actually, nodeAgg would just have to set an appropriate context before > calling fmgr to execute the aggregate's transition functions, and then > it wouldn't need an extra copy step. The results would come back in the > right context already. > > > In fact, you could even optimize the cleanup to only do free'ing if > > some expression memory was allocated. In most cases, it is not. > > Jan's stuff should already fall through pretty quickly if there's > nothing in the context, I think. Note that what we want to do between > tuples is a "context clear" of the expression context, not a "context > delete" and then "context create" a new expression context. Context > clear should be a pretty quick no-op if nothing's been allocated in that > context... > > > In fact the nodeAgg.c patch that I backed out attempted to do that, > > though because there wasn't code that checked if the Datum was > > pg_type.typbyval, it didn't work 100%. > > Right. But if we approach it this way (clear the context at appropriate > times) rather than thinking in terms of explicitly pfree'ing individual > objects, life gets much simpler. Also, if we insist on being able to > pfree individual objects inside a context, we can't use Jan's faster > allocator! Remember, the reason it is faster and lower overhead is that > it doesn't keep track of individual objects, only pools. > > I'd like to see us head in the direction of removing most of the > explicit pfree calls that exist now, and instead rely on clearing > memory contexts at appropriate times in order to manage memory. > The fewer places where we need pfree, the more contexts can be run > with the low-overhead space allocator. Also, the fewer explicit > pfrees we need, the simpler and more reliable the code gets. Tom, are you saying you agree with my approach, and I should give it a try? -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <maillist@candle.pha.pa.us> writes:
> Tom, are you saying you agree with my approach, and I should give it a
> try?
If what I said was the same as what you were thinking, then yeah ;-)
But I still think it's too late to try to fit this into 6.5, unless
the changes turn out to be way more localized than I expect.
regards, tom lane
> Bruce Momjian <maillist@candle.pha.pa.us> writes: > > Tom, are you saying you agree with my approach, and I should give it a > > try? > > If what I said was the same as what you were thinking, then yeah ;-) > > But I still think it's too late to try to fit this into 6.5, unless > the changes turn out to be way more localized than I expect. Yes, very localized. Only a few lines. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> --
> -- Query 1
> --
> select l_returnflag, l_linestatus, sum(l_quantity) as sum_qty,
> sum(l_extendedprice) as sum_base_price,
> sum(l_extendedprice*(1-l_discount)) as sum_disc_price,
> sum(l_extendedprice*(1-l_discount)*(1+l_tax)) as sum_charge,
> avg(l_quantity) as avg_qty, avg(l_extendedprice) as avg_price,
> avg(l_discount) as avg_disc, count(*) as count_order
> from lineitem
> where l_shipdate <= ('1998-12-01'::datetime - interval '90 day')::date
> group by l_returnflag, l_linestatus
> order by l_returnflag, l_linestatus;
>
>
> when I run this against 6.4.2, the postgres process grows to upwards of
> 1 GB of memory (at which point something overflows and it dumps core) -
> I watch it grow through 200 MB, 400 MB, 800 MB, dies somewhere near 1 GB
> of allocated memory).
>
Here is my first attempt at fixing the expression memory leak you
mentioned. I have run it through the regression tests, and it seems to
be harmless there.
I am interested to see if it fixes the expression leak you saw. I have
not committed this yet. I want to look at it some more.
--
Bruce Momjian | http://www.op.net/~candle
maillist@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Index: execQual.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql/src/backend/executor/execQual.c,v
retrieving revision 1.50
diff -c -r1.50 execQual.c
*** execQual.c 1999/03/20 02:07:31 1.50
--- execQual.c 1999/03/24 06:09:25
***************
*** 51,56 ****
--- 51,57 ----
#include "utils/fcache2.h"
#include "utils/mcxt.h"
#include "utils/memutils.h"
+ #include "utils/portal.h"
/*
***************
*** 65,70 ****
--- 66,74 ----
bool execConstByVal;
int execConstLen;
+ bool allocatedQualContext;
+ bool allocatedQualNesting;
+
/* static functions decls */
static Datum ExecEvalAggref(Aggref *aggref, ExprContext *econtext, bool *isNull);
static Datum ExecEvalArrayRef(ArrayRef *arrayRef, ExprContext *econtext,
***************
*** 801,814 ****
else
{
int i;
if (isDone)
*isDone = true;
for (i = 0; i < fcache->nargs; i++)
if (fcache->nullVect[i] == true)
*isNull = true;
! return (Datum) fmgr_c(&fcache->func, (FmgrValues *) argV, isNull);
}
}
--- 805,852 ----
else
{
int i;
+ Datum d;
+ char pname[64];
+ Portal qual_portal;
+ MemoryContext oldcxt;
if (isDone)
*isDone = true;
for (i = 0; i < fcache->nargs; i++)
if (fcache->nullVect[i] == true)
*isNull = true;
+
+ /*
+ * Assign adt *.c memory in separate context to prevent
+ * unbounded memory growth in large queries that use functions.
+ * We clear this memory after the qual has been completed.
+ * bjm 1999/03/24
+ */
+ strcpy(pname, "<Qual manager>");
+ qual_portal = GetPortalByName(pname);
+ if (!PortalIsValid(qual_portal))
+ {
+ qual_portal = CreatePortal(pname);
+ Assert(PortalIsValid(qual_portal));
! oldcxt = MemoryContextSwitchTo(
! (MemoryContext) PortalGetHeapMemory(qual_portal));
! StartPortalAllocMode(DefaultAllocMode, 0);
! MemoryContextSwitchTo(oldcxt);
!
! allocatedQualContext = true;
! allocatedQualNesting = 0;
! }
! allocatedQualNesting++;
!
! oldcxt = MemoryContextSwitchTo(
! (MemoryContext) PortalGetHeapMemory(qual_portal));
!
! d = (Datum) fmgr_c(&fcache->func, (FmgrValues *) argV, isNull);
!
! MemoryContextSwitchTo(oldcxt);
! allocatedQualNesting--;
! return d;
}
}
***************
*** 1354,1359 ****
--- 1392,1399 ----
{
List *clause;
bool result;
+ char pname[64];
+ Portal qual_portal;
/*
* debugging stuff
***************
*** 1387,1396 ****
break;
}
/*
* if result is true, then it means a clause failed so we
* return false. if result is false then it means no clause
! * failed so we return true.
*/
if (result == true)
return false;
--- 1427,1459 ----
break;
}
+ if (allocatedQualContext && allocatedQualNesting == 0)
+ {
+ MemoryContext oldcxt;
+
+ strcpy(pname, "<Qual manager>");
+ qual_portal = GetPortalByName(pname);
+ /*
+ * allocatedQualContext may have been improperly set from
+ * from a previous run.
+ */
+ if (PortalIsValid(qual_portal))
+ {
+ oldcxt = MemoryContextSwitchTo(
+ (MemoryContext) PortalGetHeapMemory(qual_portal));
+
+ EndPortalAllocMode();
+ StartPortalAllocMode(DefaultAllocMode, 0);
+
+ MemoryContextSwitchTo(oldcxt);
+ }
+ allocatedQualContext = false;
+ }
+
/*
* if result is true, then it means a clause failed so we
* return false. if result is false then it means no clause
! * failed so we return true. ...Yikes, who wrote that?
*/
if (result == true)
return false;
> I am interested to see if it fixes the expression leak you saw. I have > not committed this yet. I want to look at it some more. > I'm afraid that this doesn't seem to have any effect on my query. Looking at your code, I think the problem is that most of the allocations in my query are on the top part of the if statement that you modified (i.e. the == SQLlanguageId part). Below is a snippet of a trace from my query, with approximate line numbers for execQual.c with your patch applied: (execQual) language == SQLlanguageId (execQual.c:757) (execQual) execute postquel_function (execQual.c:759) (mcxt) MemoryContextAlloc 32 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 16 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 528 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 56 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 88 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 24 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 8 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 65 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 48 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 8 bytes in ** Blank Portal **-heap (execQual) else clause NOT SQLlanguageId (execQual.c:822) (execQual) install qual memory context (execQual.c:858) (execQual) exit qual context (execQual.c:862) (mcxt) MemoryContextAlloc 60 bytes in ** Blank Portal **-heap (mcxt) MemoryContextFree in ** Blank Portal **-heap freed 16 bytes (mcxt) MemoryContextFree in ** Blank Portal **-heap freed 64 bytes (mcxt) MemoryContextFree in ** Blank Portal **-heap freed 64 bytes (mcxt) MemoryContextFree in ** Blank Portal **-heap freed 528 bytes (mcxt) MemoryContextFree in ** Blank Portal **-heap freed 16 bytes (execQual) return from postquel_function (execQual.c:764) (execQual) return from ExecEvalFuncArgs (execQual.c:792) (execQual) else clause NOT SQLlanguageId (execQual.c:822) (execQual) install qual memory context (execQual.c:858) (execQual) exit qual context (execQual.c:862) (mcxt) MemoryContextAlloc 108 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 108 bytes in ** Blank Portal **-heap (mcxt) MemoryContextFree in ** Blank Portal **-heap freed 128 bytes (execQual) else clause NOT SQLlanguageId (execQual.c:822) (execQual) install qual memory context (execQual.c:858) (mcxt) MemoryContextAlloc 8 bytes in <Qual manager>-heap (execQual) exit qual context (execQual.c:862) <pattern repeats> (execQual) language == SQLlanguageId (execQual.c:757) (execQual) execute postquel_function (execQual.c:759) (mcxt) MemoryContextAlloc 32 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 16 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 528 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 56 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 88 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 24 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 8 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 65 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 48 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 8 bytes in ** Blank Portal **-heap (execQual) else clause NOT SQLlanguageId (execQual.c:822) (execQual) install qual memory context (execQual.c:858) (execQual) exit qual context (execQual.c:862) (mcxt) MemoryContextAlloc 60 bytes in ** Blank Portal **-heap (mcxt) MemoryContextFree in ** Blank Portal **-heap freed 16 bytes (mcxt) MemoryContextFree in ** Blank Portal **-heap freed 64 bytes (mcxt) MemoryContextFree in ** Blank Portal **-heap freed 64 bytes (mcxt) MemoryContextFree in ** Blank Portal **-heap freed 528 bytes (mcxt) MemoryContextFree in ** Blank Portal **-heap freed 16 bytes (execQual) return from postquel_function (execQual.c:764) (execQual) return from ExecEvalFuncArgs (execQual.c:792) (execQual) else clause NOT SQLlanguageId (execQual.c:822) (execQual) install qual memory context (execQual.c:858) (execQual) exit qual context (execQual.c:862) (mcxt) MemoryContextAlloc 108 bytes in ** Blank Portal **-heap (mcxt) MemoryContextAlloc 108 bytes in ** Blank Portal **-heap (mcxt) MemoryContextFree in ** Blank Portal **-heap freed 128 bytes (execQual) else clause NOT SQLlanguageId (execQual.c:822) (execQual) install qual memory context (execQual.c:858) (mcxt) MemoryContextAlloc 8 bytes in <Qual manager>-heap (execQual) exit qual context (execQual.c:862) the MemoryContext lines give the name of the portal where each allocation is happening - you see that your Qual manager only captures a very small number (one) of the allocations, the rest are in the upper part of the if statement. Note that I also placed a printf next to your EndPortalAllocMode() and StartPortalAllocMode() fix in ExecQual() - I believe this is what is supposed to clear the portal and free the memory - and that printf never appears in the above trace. Sorry if the trace is a little confusing, but I hope that it helps you zero in. Erik