From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
Subject: | Re: GiST buffering build, bug in levelStep calculation |
Date: | 2012-05-29 19:42:30 |
Message-ID: | 17929.1338320550@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> This is because of this rather complex calculation here:
>> while (
>> /* subtree must fit in cache (with safety factor of 4) */
>> (1 - pow(avgIndexTuplesPerPage, (double) (levelStep + 1))) / (1 - avgIndexTuplesPerPage) < effective_cache_size / 4
>> &&
>> /* each node in the lowest level of a subtree has one page in memory */
>> (pow(maxIndexTuplesPerPage, (double) levelStep) < (maintenance_work_mem * 1024) / BLCKSZ)
>> )
> What happens here is that the calculation of (maintenance_work_mem *
> 1024) / BLCKSZ overflows the 32 bit signed integer type used there, if
> maintenance_work_mem >= 2GB. I think we should be doing all these
> calculations in double.
Given the use of pow(), I concur with changing the whole calculation to
double. Just as a remark, the correct way to code that sort of thing
normally is (maintenance_work_mem * 1024L) / BLCKSZ, which avoids
overflow because guc.c limits MAX_KILOBYTES to ensure it won't overflow
a long. (Given that we are now supporting Win64 where long is narrower
than size_t, we might want to revisit this coding rule eventually, but
it's not high on my priority list.)
While I'm looking at this, is the first test involving
effective_cache_size bulletproof either? In particular, is
avgIndexTuplesPerPage clamped to be strictly greater than 1?
And for that matter, is either test sane from a units standpoint?
It seems to me that maxIndexTuplesPerPage would have units of
1/blocks, which is pretty dubious to be comparing to a block count
even disregarding the power function.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | james | 2012-05-29 19:46:14 | Fake async rep target |
Previous Message | Bruce Momjian | 2012-05-29 19:40:43 | Re: pg_upgrade libraries check |