pgsql: Fix accounting of memory needed for merge heap.

Started by Heikki Linnakangasover 9 years ago4 messagescomitters
Jump to latest
#1Heikki Linnakangas
heikki.linnakangas@enterprisedb.com

Fix accounting of memory needed for merge heap.

We allegedly allocated all remaining memory for the read buffers of the
sort tapes, but we allocated the merge heap only after that. That means
that the allocation of the merge heap was guaranteed to go over the memory
limit. Fix by allocating the merge heap first. This makes little difference
in practice, because the merge heap is tiny, but let's tidy.

While we're at it, add a safeguard for the case that we are already over
the limit when allocating the read buffers. That shouldn't happen, but
better safe than sorry.

The memory accounting error was reported off-list by Peter Geoghegan.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/f7d54f4f7ddf72bf4db1783890b058e758b4b894

Modified Files
--------------
src/backend/utils/sort/tuplesort.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

In reply to: Heikki Linnakangas (#1)
Re: pgsql: Fix accounting of memory needed for merge heap.

On Thu, Dec 8, 2016 at 12:20 AM, Heikki Linnakangas
<heikki.linnakangas@iki.fi> wrote:

While we're at it, add a safeguard for the case that we are already over
the limit when allocating the read buffers. That shouldn't happen, but
better safe than sorry.

I think you should 's/Min/Max/' where the new limit is applied. Also
suggest that the subsequent USEMEM() call have
"state->read_buffer_size * numInputTapes" as its argument, rather than
state->availMem, just to be neat.

Thanks
--
Peter Geoghegan

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#3Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Peter Geoghegan (#2)
Re: pgsql: Fix accounting of memory needed for merge heap.

On 12/08/2016 10:49 PM, Peter Geoghegan wrote:

On Thu, Dec 8, 2016 at 12:20 AM, Heikki Linnakangas
<heikki.linnakangas@iki.fi> wrote:

While we're at it, add a safeguard for the case that we are already over
the limit when allocating the read buffers. That shouldn't happen, but
better safe than sorry.

I think you should 's/Min/Max/' where the new limit is applied. Also
suggest that the subsequent USEMEM() call have
"state->read_buffer_size * numInputTapes" as its argument, rather than
state->availMem, just to be neat.

D'oh, you're right, of course. Fixed, thanks for the vigilance!

- Heikki

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

In reply to: Heikki Linnakangas (#3)
Re: pgsql: Fix accounting of memory needed for merge heap.

On Thu, Dec 8, 2016 at 1:07 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

D'oh, you're right, of course. Fixed, thanks for the vigilance!

I've made exactly the same mistake myself before. :-)

--
Peter Geoghegan

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers