On Sat, Jun 22, 2013 at 11:36:45AM +0100, Simon Riggs wrote: > On 13 May 2013 15:26, Noah Misch <n...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
