Re: MemoryContextAllocHuge(): selectively bypassing MaxAllocSize

From: Noah Misch <noah(at)leadboat(dot)com>
To: Simon Riggs <simon(at)2ndQuadrant(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: MemoryContextAllocHuge(): selectively bypassing MaxAllocSize
Date: 2013-06-24 16:36:20
Message-ID: 20130624163620.GB835122@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Jun 22, 2013 at 11:36:45AM +0100, Simon Riggs wrote:
> On 13 May 2013 15:26, Noah Misch <noah(at)leadboat(dot)com> wrote:

> I'm concerned that people will accidentally use MaxAllocSize. Can we
> put in a runtime warning if someone tests AllocSizeIsValid() with a
> larger value?

I don't see how we could. To preempt a repalloc() failure, you test with
AllocSizeIsValid(); testing a larger value is not a programming error.

> > To demonstrate, I put this to use in
> > tuplesort.c; the patch also updates tuplestore.c to keep them similar. Here's
> > the trace_sort from building the pgbench_accounts primary key at scale factor
> > 7500, maintenance_work_mem = '56GB'; memtuples itself consumed 17.2 GiB:
> >
> > LOG: internal sort ended, 48603324 KB used: CPU 75.65s/305.46u sec elapsed 391.21 sec
> >
> > Compare:
> >
> > LOG: external sort ended, 1832846 disk blocks used: CPU 77.45s/988.11u sec elapsed 1146.05 sec
>
> Cool.
>
> I'd like to put in an explicit test for this somewhere. Obviously not
> part of normal regression, but somewhere, at least, so we have
> automated testing that we all agree on. (yes, I know we don't have
> that for replication/recovery yet, but thats why I don't want to
> repeat that mistake).

Probably the easiest way to test from nothing is to run "pgbench -i -s 7500"
under a high work_mem. I agree that an automated test suite dedicated to
coverage of scale-dependent matters would be valuable, though I'm disinclined
to start one in conjunction with this particular patch.

> > The comment at MaxAllocSize said that aset.c expects doubling the size of an
> > arbitrary allocation to never overflow, but I couldn't find the code in
> > question. AllocSetAlloc() does double sizes of blocks used to aggregate small
> > allocations, so maxBlockSize had better stay under SIZE_MAX/2. Nonetheless,
> > that expectation does apply to dozens of repalloc() users outside aset.c, and
> > I preserved it for repalloc_huge(). 64-bit builds will never notice, and I
> > won't cry for the resulting 2 GiB limit on 32-bit.
>
> Agreed. Can we document this for the relevant parameters?

I attempted to cover most of that in the comment above MaxAllocHugeSize, but I
did not mention the maxBlockSize constraint. I'll add an
Assert(AllocHugeSizeIsValid(maxBlockSize)) and a comment to
AllocSetContextCreate(). Did I miss documenting anything else notable?

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Berkus 2013-06-24 16:57:45 Re: [9.4 CF 1] The Commitfest Slacker List
Previous Message Noah Misch 2013-06-24 16:33:18 Re: MemoryContextAllocHuge(): selectively bypassing MaxAllocSize