Kyotaro HORIGUCHI <[email protected]> wrote:
> At Thu, 12 Mar 2015 15:27:37 -0700, Peter Geoghegan <[email protected]> wrote:
>> On Sat, Feb 14, 2015 at 4:19 PM, Kevin Grittner <[email protected]> wrote:
>>> At some point we could consider building on this patch to
>>> recheck index conditions for heap access when a non-MVCC
>>> snapshot is used, check the visibility map for referenced heap
>>> pages when the TIDs are read for an index-only scan, and/or
>>> skip LP_DEAD hinting for non-WAL-logged indexes. But all those
>>> are speculative future work; this is a conservative
>>> implementation that just didn't modify pinning where there were
>>> any confounding factors.
>> I think you should call out those "confounding factors" in the
>> README. It's not hard to piece them together from
>> _bt_drop_lock_and_maybe_pin(), but I think you should anyway.
OK:
https://github.com/kgrittn/postgres/commit/f5f59ded30b114ac83b90a00ba1fa5ef490b994e
>> Don't use BUFFER_LOCK_SHARE -- use BT_READ, as the existing
>> nbtree LockBuffer() callers do. You're inconsistent about that
>> in V3.
>
> I agree with you. It looks the only point where it is used.
OK:
https://github.com/kgrittn/postgres/commit/76118b58be819ed5e68569c926d0222bc41640ea
> Addition to that, the commnet just above the point methioned
> above quizes me.
>
>> /* XXX: Can walking left be lighter on the locking and pins? */
>> if (BTScanPosIsPinned(so->currPos))
>> LockBuffer(so->currPos.buf, BUFFER_LOCK_SHARE);
>> else
>> so->currPos.buf = _bt_getbuf(rel, so->currPos.currPage, BT_READ);
>
> I'm happy if I could read the meaming of the comment more
> clearly. I understand that it says that you want to remove the
> locking (and pinning), but can't now because the simultaneous
> splitting of the left page would break something. I'd like to see
> it clearer even for me either I am correct or not..
Does this clear it up?:
https://github.com/kgrittn/postgres/commit/22066fc161a092e800e4c1e853136c4513f8771b
Since there are no changes that would affect the compiled code
here, I'm not posting a new patch yet. I'll do that once things
seem to have settled down a bit more.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
