Horton

Re: Patch: Write Amplification Reduction Method (WARM)

Pavan Deolasee <pavan.deolasee@gmail.com>

View thread View on postgr.es
From: Pavan Deolasee <pavan.deolasee@gmail.com>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Bruce Momjian <bruce@momjian.us>, Jaime Casanova <jaime.casanova@2ndquadrant.com>, Haribabu Kommi <kommi.haribabu@gmail.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-03-20T15:09:43Z
Lists: pgsql-hackers 
On Tue, Mar 14, 2017 at 7:17 AM, Alvaro Herrera <alvherre@2ndquadrant.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

Commits

  1. Implement SortSupport for macaddr data type

  2. Simplify check of modified attributes in heap_update

  3. Remove direct uses of ItemPointer.{ip_blkid,ip_posid}

  4. Fix CatalogTupleInsert/Update abstraction for case of shared indstate.

  5. Provide CatalogTupleDelete() as a wrapper around simple_heap_delete().

  6. Band-aid fix for incorrect use of view options as StdRdOptions.

  7. Update visibility map in the second phase of vacuum.

  8. Avoid having two copies of the HOT-chain search logic.

  9. Postgres95 1.01 Distribution - Virgin Sources