From: | Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Jaime Casanova <jaime(dot)casanova(at)2ndquadrant(dot)com>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Patch: Write Amplification Reduction Method (WARM) |
Date: | 2017-03-20 15:09:43 |
Message-ID: | CABOikdN79xE=Rw=GB8_xsuShU1WXCED1ZU5H6sKxb33vMzsYRQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Mar 14, 2017 at 7:17 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
wrote:
> > @@ -234,6 +236,21 @@ index_beginscan(Relation heapRelation,
> > scan->heapRelation = heapRelation;
> > scan->xs_snapshot = snapshot;
> >
> > + /*
> > + * If the index supports recheck, make sure that index tuple is
> saved
> > + * during index scans.
> > + *
> > + * XXX Ideally, we should look at all indexes on the table and
> check if
> > + * WARM is at all supported on the base table. If WARM is not
> supported
> > + * then we don't need to do any recheck.
> RelationGetIndexAttrBitmap() does
> > + * do that and sets rd_supportswarm after looking at all indexes.
> But we
> > + * don't know if the function was called earlier in the session
> when we're
> > + * here. We can't call it now because there exists a risk of
> causing
> > + * deadlock.
> > + */
> > + if (indexRelation->rd_amroutine->amrecheck)
> > + scan->xs_want_itup = true;
> > +
> > return scan;
> > }
>
> I didn't like this comment very much. But it's not necessary: you have
> already given relcache responsibility for setting rd_supportswarm. The
> only problem seems to be that you set it in RelationGetIndexAttrBitmap
> instead of RelationGetIndexList, but it's not clear to me why. I think
> if the latter function is in charge, then we can trust the flag more
> than the current situation.
I looked at this today. AFAICS we don't have access to rd_amroutine in
RelationGetIndexList since we don't actually call index_open() in that
function. Would it be safe to do that? I'll give it a shot, but thought of
asking here first.
Thanks,
Pavan
From | Date | Subject | |
---|---|---|---|
Next Message | Alexander Korotkov | 2017-03-20 15:12:04 | Re: PinBuffer() no longer makes use of strategy |
Previous Message | Teodor Sigaev | 2017-03-20 15:09:37 | Re: PinBuffer() no longer makes use of strategy |