Обсуждение: pgsql: Fix accounting of memory needed for merge heap.
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(-)
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
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
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
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
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