Re: tuplesort memory usage: grow_memtuples

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Peter Geoghegan <peter(at)2ndquadrant(dot)com>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Greg S <stark(at)mit(dot)edu>
Subject: Re: tuplesort memory usage: grow_memtuples
Date: 2012-11-15 18:13:07
Message-ID: CA+TgmobLk6foObGzh-FnAdMCj9JbDuct2sXrSjBy4CA0fQ073Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Nov 15, 2012 at 12:14 PM, Peter Geoghegan <peter(at)2ndquadrant(dot)com> wrote:
> On 15 November 2012 16:09, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> [Lots of complicated commentary on tuplesort variables]
>> Whew. In the attached version, I
>> updated the comment to reflect this, and also reversed the order of
>> the if/else block to put the shorter and more common case first, which
>> I think is better style.
>
> Fine by me.
>
> In conversation with Greg Stark in Prague, he seemed to think that
> there was an integer overflow hazard in v3, which is, in terms of the
> machine code it will produce, identical to your revision. He pointed
> this out to me when we were moving between sessions, and I only
> briefly looked over his shoulder, so while I don't want to
> misrepresent what he said, I *think* he said that this could overflow:
>
> + newmemtupsize = memtupsize * allowedMem / memNowUsed;

Ah, yeah. I wondered in passing about that but forgot to follow up on
it. The problem specifically is that the intermediate result
memtupsize * newmemtuples might overflow. I believe that the old
memtupsize can never be more than 2^26 bytes, because the allocation
limit is 1GB and each SortTuple is 16 bytes. So if this is done as a
32-bit calculation, we'll potentially overflow if allowedMem is more
than 64 bytes i.e. almost for sure. If it is done as a 64-byte
calculation we'll potentially overflow if allowedMem is more than 2^38
bytes = 256GB. The actual declared type is "long" which I assume is
probably 64 bits at least for most people these days, but maybe not
for everyone, and even 256GB is not necessarily safe. The highest
value for work_mem I can set here is 2047GB. So I think there is an
issue here, unless there's something wrong with my analysis.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Janes 2012-11-15 18:19:08 pg_ctl reload -o "...."
Previous Message Cédric Villemain 2012-11-15 18:06:48 Re: another idea for changing global configuration settings from SQL