Re: Reduce pinning in btree indexes

Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> At Thu, 12 Mar 2015 15:27:37 -0700, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>> On Sat, Feb 14, 2015 at 4:19 PM, Kevin Grittner <kgrittn(at)ymail(dot)com> 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