Re: Patch: Write Amplification Reduction Method (WARM)

On Wed, Mar 8, 2017 at 12:14 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Alvaro Herrera wrote:
>> Here's a rebased set of patches.  This is the same Pavan posted; I only
>> fixed some whitespace and a trivial conflict in indexam.c, per 9b88f27cb42f.
>
> Jaime noted that I forgot the attachments.  Here they are

If I recall correctly, the main concern about 0001 was whether it
might negatively affect performance, and testing showed that, if
anything, it was a little better. Does that sound right?

Regarding 0002, I think this could use some documentation someplace
explaining the overall theory of operation.  README.HOT, maybe?

+     * Most often and unless we are dealing with a pg-upgraded cluster, the
+     * root offset information should be cached. So there should not be too
+     * much overhead of fetching this information. Also, once a tuple is
+     * updated, the information will be copied to the new version. So it's not
+     * as if we're going to pay this price forever.

What if a tuple is updated -- presumably clearing the
HEAP_LATEST_TUPLE on the tuple at the end of the chain -- and then the
update aborts?  Then we must be back to not having this information.

One overall question about this patch series is how we feel about
using up this many bits.  0002 uses a bit from infomask, and 0005 uses
a bit from infomask2.  I'm not sure if that's everything, and then I
think we're steeling some bits from the item pointers, too.  While the
performance benefits of the patch sound pretty good based on the test
results so far, this is definitely the very last time we'll be able to
implement a feature that requires this many bits.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company