On 15.02.2012 10:18, Alexander Korotkov wrote:
> On Wed, Feb 15, 2012 at 2:54 AM, Tom Lane<tgl@sss.pgh.pa.us> wrote:
>
>> Alexander Korotkov<aekorotkov@gmail.com> writes:
>>> ITSM, I found the problem. This piece of code is triggering an error. It
>>> assumes each page of corresponding to have initialized buffer. That
>> should
>>> be true because we're inserting index tuples from up to down while
>>> splits propagate from down to up.
>>> But this assumptions becomes false we turn buffer off in the root page.
>> So,
>>> root page can produce pages without initialized buffers when splits.
>>
>> Hmm ... can we tighten the error check rather than just remove it? It
>> feels less than safe to assume that a hash-entry-not-found condition
>> *must* reflect a corner-case situation like that. At the very least
>> I'd like to see it verify that we'd turned off buffering before deciding
>> this is OK. Better, would it be practical to make dummy entries in the
>> hash table even after turning buffers off, so that the logic here
>> becomes
>>
>> if (!found) error;
>> else if (entry is dummy) return without doing anything;
>> else proceed;
>>
>> regards, tom lane
>>
>
> Ok, there is another patch fixes this problem. Instead of error triggering
> remove it adds empty buffers on root page split if needed.
Actually, I think it made sense to simply do nothing if the buffer
doesn't exist. The algorithm doesn't require that all the buffers must
exist at all times. It is quite accidental that whenever we call
gistRelocateBuildBuffersOnSplit(), the page must already have its buffer
created (and as we found out, the assumption doesn't hold after a root
split, anyway). Also, we talked earlier that it would be good to destroy
buffers that become completely empty, to save memory. If we do that,
we'd have to remove that check anyway.
So, I think we should go with your original fix and simply do nothing in
gistRelocateBuildBuffersOnSplit() if the page doesn't have a buffer.
Moreover, if the page has a buffer but it's empty,
gistRelocateBuildBuffersOnSplit() doesn't need to create buffers for the
new sibling pages. In the final emptying phase, that's a waste of time,
the buffers we create will never be used, and even before that I think
it's better to create the buffers lazily.
-- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com