On Thu, Dec 15, 2016 at 11:33 AM, Amit Kapila <amit.kapila16@gmail.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. Aside
from that there's no reason for it to touch wbuf. If you fix it like
that then you should be able to avoid this rather ugly wart:
* XXX Here, we are moving to next overflow page for writing without * ensuring if the previous write page is
full. This is annoying, but * should not hurt much in practice as that space will anyway be consumed * by future
inserts.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company