Re: Reviewing freeze map code

Hi,

On 2016-06-06 15:16:10 -0400, Robert Haas wrote:
> On Mon, Jun 6, 2016 at 2:35 PM, Andres Freund <andres@anarazel.de> wrote:
> >> > Why would they have to write the complex query?  Wouldn't they just
> >> > need to run that we wrote for them?
> >
> > Then write that query. Verify that that query performs halfway
> > reasonably fast. Document that it should be run against databases after
> > subjecting them to tests. That'd address my concern as well.
> 
> You know, I am starting to lose a teeny bit of patience here.

Same here.


> I do appreciate you reviewing this code, very much, and genuinely, and
> it would be great if more people wanted to review it.

> But this kind of reads like you think that I'm being a jerk, which I'm
> trying pretty hard not to be

I don't think you're a jerk. But I am loosing a good bit of my patience
here. I've posted these issues a month ago, and for a long while the
only thing that happened was bikeshedding about the name of something
that wasn't even decided to happen yet (obviously said bikeshedding
isn't your fault).


> and like you have the right to tell assign me arbitrary work, which I
> think you don't.

It's not like adding a parameter for this would be a lot of work,
there's even a patch out there.  I'm getting impatient because I feel
the issue of this critical feature not being testable is getting ignored
and/or played down.  And then sidetracked into a general "let's add a
database consistency checker" type discussion. Which we need, but won't
get in 9.6.

If you say: "I agree with the feature in principle, but I don't want to
spend time to review/commit it." - ok, that's fair enough. But at the
moment that isn't what I'm reading between the lines.


> If you want to have a
> reasonable conversation about what the options are for making this
> better, great.

Yes, I want that.


> If you want to me to do some work to help improve things on a patch I
> committed, that is 100% fair.  But I don't know what I did to earn
> this response which, to me, reads as rather demanding and rather
> exasperated.

I don't think it's absurd to make some demands on the committer of a
impact-heavy feature, about at least finding a realistic path towards
the new feature being realistically testable.  This is a scary (but
*REALLY IMPORTANT*) patch, and I don't understand why it's ok that we
can't push a it through a couple wraparounds under high concurrency, and
easily verify that the freeze map is in sync with the actual data.

And yes, I *am* exasperated, that I'm the only one that appears to be
scared by the lack of that capability.  I think the feature is in a
*lot* better shape than multixacts, but it certainly has the potential
to do even more damage in ways that'll essentially be unrecoverable.

Andres