On 15.02.2012 10:18, Alexander Korotkov wrote:
On Wed, Feb 15, 2012 at 2:54 AM, Tom Lane<t...@sss.pgh.pa.us>  wrote:

Alexander Korotkov<aekorot...@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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to