Re: Bug in 9.6 tuplesort batch memory growth logic

From: Peter Geoghegan <pg(at)heroku(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Bug in 9.6 tuplesort batch memory growth logic
Date: 2016-09-06 15:40:34
Message-ID: CAM3SWZTFCe4G4wGC0XQX45_G3oTW1HRoNTjoj2ULYXzVcJAYuA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Sep 6, 2016 at 6:17 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> It doesn't seem to me that this limit has anything to do with anything,
> and the comment claiming only that it's "noncritical" isn't helping.
> If the point is to prevent the later LACKMEM check from failing, then
> certainly there is something critical somewhere. I'd rather see this
> explained as "we need at least X, but we choose to require at least Y
> to avoid repalloc thrashing".

You've not understood the problem at all. The only thing that's
critical is that the calculation not fail at all, through a later
availMem that is < 0 (i.e. a LACKMEM() condition).

> And maybe the code should use Max(X,Y)
> rather than blindly assuming that ALLOCSET_DEFAULT_INITSIZE exceeds
> whatever the true minimum is.

The true minimum is 0, so that seems like a safe bet. Comments point
this out directly, and that we are not reliant on having any
particular amount of memory available, even enough for one tuple (the
overflow mechanism will later save us).

I knew that the patch would be criticized for still allowing a useless
palloc, and some threshold was needed. I also knew any choice of
constant would be criticized (e.g., "that's voodoo"), so pointed out
specifically that it was non-critical.

What threshold would you use?
--
Peter Geoghegan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2016-09-06 15:41:08 Re: ICU integration
Previous Message Peter Eisentraut 2016-09-06 15:37:37 Re: ICU integration