|From:||Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>|
|Subject:||Re: Reduce pinning in btree indexes|
|Views:||Raw Message | Whole Thread | Download mbox|
Thank you for rewriting.
I was satisfied with the current patch (Head of btnopin on your
github repos). I have no further comment on the functions. Peter
might have a comment on the README description but I suppose I
can push this to the committers.
I attached the your latest patch to this mail as
bt-nopin-v4.patch for now. Please check that there's no problem
- By this patch, index scan becomes to release buffer pins while
fetching index tuples in a page, so it should reduce the chance
of index scans with long duration to block vacuum, because
vacuums now can easily overtake the current position of an
index scan. I didn't actually measured how effective it is,
- It looks to work correctly for scan in both direction with
simultaneous page splitting and vacuum.
- It makes no performance deterioration, on the contrary it
accelerates index scans. It seems to be because of removal of
lock and unlock surrounding _bt_steppage in bt_next.
- It applies cleanly on the current head on the master branch.
- It has enough intelligible comments and README descriptions.
- An inrelevant typo fix is made in buffers/README by this
patch. But it doesn't seem necessary to be separated.
At Fri, 13 Mar 2015 15:08:25 +0000 (UTC), Kevin Grittner <kgrittn(at)ymail(dot)com> wrote in <848652045(dot)4044965(dot)1426259305233(dot)JavaMail(dot)yahoo(at)mail(dot)yahoo(dot)com>
> 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:
> >> 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.
It looks perfect for me. Do you have any further request on this,
> >> 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.
> > 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?:
Thank you for the labor. It also looks perfect.
> 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.
NTT Open Source Software Center
|Next Message||Fabien COELHO||2015-03-16 07:02:46||Re: PATCH: pgbench - merging transaction logs|
|Previous Message||Kouhei Kaigai||2015-03-16 06:50:25||Re: Custom/Foreign-Join-APIs (Re: [v9.5] Custom Plan API)|