Re: Patch: Write Amplification Reduction Method (WARM)

On Fri, Mar 31, 2017 at 11:16 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Fri, Mar 31, 2017 at 10:24 AM, Pavan Deolasee
> <pavan(dot)deolasee(at)gmail(dot)com> wrote:
> > Having worked on it for some time now, I can say that WARM uses pretty
> much
> > the same infrastructure that HOT uses for cleanup/pruning tuples from the
> > heap. So the risk of having a bug which can eat your data from the heap
> is
> > very low. Sure, it might mess up with indexes, return duplicate keys, not
> > return a row when it should have. Not saying they are not bad bugs, but
> > probably much less severe than someone removing live rows from the heap.
>
> Yes, that's true. If there's nothing wrong with the way pruning
> works, then any other problem can be fixed by reindexing, I suppose.
>
>
Yeah, I think so.

> I'm not generally a huge fan of on-off switches for things like this,
> but I know Simon likes them. I think the question is how much they
> really insulate us from bugs. For the hash index patch, for example,
> the only way to really get insulation from bugs added in this release
> would be to ship both the old and the new code in separate index AMs
> (hash, hash2). The code has been restructured so much in the process
> of doing all of this that any other form of on-off switch would be
> pretty hit-or-miss whether it actually provided any protection.
>
> Now, I am less sure about this case, but my guess is that you can't
> really have this be something that can be flipped on and off for a
> table. Once a table has any WARM updates in it, the code that knows
> how to cope with that has to be enabled, and it'll work as well or
> poorly as it does.

That's correct. Once enabled, we will need to handle the case of two index
pointers pointing to the same root. The only way to get rid of that is
probably do a complete rewrite/reindex, I suppose. But I was mostly talking
about immutable flag at table creation time as rightly guessed.

> Now, I understand you to be suggesting a flag at
> table-creation time that would, maybe, be immutable after that, but
> even then - are we going to run completely unmodified 9.6 code for
> tables where that's not enabled, and only go through any of the WARM
> logic when it is enabled? Doesn't sound likely. The commits already
> made from this patch series certainly affect everybody, and I can't
> see us adding switches that bypass
> ce96ce60ca2293f75f36c3661e4657a3c79ffd61 for example.

I don't think I am going to claim that either. But probably only 5% of the
new code would then be involved. Which is a lot less and a lot more
manageable. Having said that, I think if we at all do this, we should only
do it based on our experiences in the beta cycle, as a last resort. Based
on my own experiences during HOT development, long running pgbench tests,
with several concurrent clients, subjected to multiple AV cycles and
periodic consistency checks, usually brings up issues related to heap
corruption. So my confidence level is relatively high on that part of the
code. That's not to suggest that there can't be any bugs.

Obviously then there are other things such as regression to some workload
or additional work required by vacuum etc. And I think we should address
them and I'm fairly certain we can do that. It may not happen immediately,
but if we provide right knobs, may be those who are affected can fall back
to the old behaviour or not use the new code at all while we improve things
for them. Some of these things I could have already implemented, but
without a clear understanding of whether the feature will get in or not,
it's hard to keep putting infinite efforts into the patch. All
non-committers go through that dilemma all the time, I'm sure.

Thanks,
Pavan

--
Pavan Deolasee http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services