Date: Wed, 1 Feb 2017 18:31:44 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Pavan Deolasee <pavan.deolasee@gmail.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Patch: Write Amplification Reduction Method (WARM)
Message-ID: <20170201213144.xb64zuem6azqntrs@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <27502.1485981379@sss.pgh.pa.us>
User-Agent: NeoMutt/20170113 (1.7.2)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

Tom Lane wrote:

> The source of both of those problems is that in some places, we
> did CatalogOpenIndexes and then used the CatalogIndexState for
> multiple tuple inserts/updates before doing CatalogCloseIndexes.
> The patch dealt with these either by not touching them, just
> leaving the simple_heap_insert/update calls in place (thus failing
> to create any abstraction), or by blithely ignoring the optimization
> and doing s/simple_heap_insert/CatalogTupleInsert/ anyway.  For example,
> in inv_api.c we are now doing a CatalogOpenIndexes/CatalogCloseIndexes
> cycle for each chunk of the large object ... and just to add insult to
> injury, the now-useless open/close calls outside the loop are still there.

Ouch.  You're right, I missed that.

> I think what we ought to do about this is invent additional API
> functions, say
> 
> Oid CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup,
>                                CatalogIndexState indstate);
> void CatalogTupleUpdateWithInfo(Relation heapRel, ItemPointer otid,
>                                 HeapTuple tup, CatalogIndexState indstate);
> 
> and use these in place of simple_heap_foo plus CatalogIndexInsert
> in the places where this optimization had been applied.

This looks reasonable enough to me.

> An alternative but much more complicated fix would be to get rid of
> the necessity for callers to worry about this at all, by caching
> a CatalogIndexState in the catalog's relcache entry.  That might be
> worth doing eventually (because it would allow sharing index info
> collection across unrelated operations) but I don't want to do it today.

Hmm, interesting idea.  No disagreement on postponing.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers