Re: Hash Indexes

On Fri, Dec 16, 2016 at 9:57 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Dec 15, 2016 at 11:33 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> Attached are the two patches on top of remove-hash-wrtbuf. Patch
>> fix_dirty_marking_v1.patch allows to mark the buffer dirty in one of
>> the corner cases in _hash_freeovflpage() and avoids to mark dirty
>> without need in _hash_squeezebucket(). I think this can be combined
>> with remove-hash-wrtbuf patch. fix_lock_chaining_v1.patch fixes the
>> chaining behavior (lock next overflow bucket page before releasing the
>> previous bucket page) was broken in _hash_freeovflpage(). These
>> patches can be applied in series, first remove-hash-wrtbuf, then
>> fix_dirst_marking_v1.patch and then fix_lock_chaining_v1.patch.
>
> I committed remove-hash-wrtbuf and fix_dirty_marking_v1 but I've got
> some reservations about fix_lock_chaining_v1. ISTM that the natural
> fix here would be to change the API contract for _hash_freeovflpage so
> that it doesn't release the lock on the write buffer. Why does it
> even do that? I think that the only reason why _hash_freeovflpage
> should be getting wbuf as an argument is so that it can handle the
> case where wbuf happens to be the previous block correctly.
>

Yeah, as of now that is the only case, but for WAL patch, I think we
need to ensure that the action of moving all the tuples to the page
being written and the overflow page being freed needs to be logged
together as an atomic operation. Now apart from that, it is
theoretically possible that write page will remain locked for multiple
overflow pages being freed (when the page being written has enough
space that it can accommodate tuples from multiple overflow pages). I
am not sure if it is worth worrying about such a case because
practically it might happen rarely. So, I have prepared a patch to
retain a lock on wbuf in _hash_freeovflpage() as suggested by you.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com