On Sat, Jun 22, 2013 at 11:36:45AM +0100, Simon Riggs wrote:
> On 13 May 2013 15:26, Noah Misch <noah@leadboat.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