Re: Patch: Write Amplification Reduction Method (WARM)

Hi,

On 2017-04-08 23:36:13 +0530, Pavan Deolasee wrote:
> On Wed, Apr 5, 2017 at 11:57 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> > On 2017-04-05 09:36:47 -0400, Robert Haas wrote:
> > > By the way, the "Converting WARM chains back to HOT chains" section of
> > > README.WARM seems to be out of date. Any chance you could update that
> > > to reflect the current state and thinking of the patch?
> >
> > I propose we move this patch to the next CF. That shouldn't prevent you
> > working on it, although focusing on review of patches that still might
> > make it wouldn't hurt either.
> >
> >
> Thank you all for the reviews, feedback, tests, criticism. And apologies
> for keep pushing it till the last minute even though it was clear to me
> quite some time back the patch is not going to make it.

What confuses me about that position is that people were advocating to
actually commit till literally hours before the CF closed.

> But if I'd given
> up, it would have never received whatever little attention it got. The only
> thing that disappoints me is that the patch was held back on no strong
> technical grounds - at least none were clear to me. There were concerns
> about on-disk changes etc, but most on-disk changes were known for 7 months
> now. Reminds me of HOT development, when it would not receive adequate
> feedback for quite many months, probably for very similar reasons - complex
> patch, changes on-disk format, risky, even though performance gains were
> quite substantial. I was much more hopeful this time because we have many
> more experts now as compared to then, but we probably have equally more
> amount of complex patches to review/commit.

I don't think it's realistic to expect isolated in-depth review of
on-disk changes, when the rest of the patch isn't in a close-to-ready
shape. The likelihood that further work on the patch invalidates such
in-depth review is significant. It's not like only minor details changed
in the last few months.

I do agree that it's hard to get qualified reviewers on bigger patches.
But I think part of the reaction to that has to be active work on that
front: If your patch needs reviews by committers or other topical
experts, you need to explicitly reach out. There's a lot of active
threads, and nobody has time to follow all of them in sufficient detail
to know that certain core parts of an actively developed patch are ready
for review. Offer tit-for-tat reviews. Announce that your patch is
ready, that you're only waiting for review. Post a summary of open
questions...

> Finally, my apologies for not spending enough time reviewing other
> patches. I know its critical, and I'll try to improve on that.

I do find it a more than a bit ironic to lament early lack of attention
to your patch, while also being aware of not having done much review.
This can only scale if everyone reviews each others patches, not if
there's a few individuals that have to review everyones patches.

Greetings,

Andres Freund