Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote: > At Thu, 12 Mar 2015 15:27:37 -0700, Peter Geoghegan <p...@heroku.com> wrote: >> On Sat, Feb 14, 2015 at 4:19 PM, Kevin Grittner <kgri...@ymail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
