Re: what to revert

On Wed, May 4, 2016 at 1:42 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>> Surely nobody here is blind to the fact that holding back xmin often
>> creates a lot of bloat for no reason - many or all of the retained old
>> row versions may never be accessed.
>
> Definitely. I *like* the feature.

+1.

>> I do not think it's a show-stopper if you wish he'd worked harder on
>> the performance under heavy concurrency with very short transactions.
>> You could have raised that issue at any time, but instead waited until
>> after feature freeze.
>
> Sorry, I don't buy that argument. There were senior community people
> giving feedback on the patch, the potential for performance regressions
> wasn't called out, the patch was updated pretty late in the CF. I find
> it really scary that review didn't call out the potential for
> regressions here, at the very least the ones with the feature disabled
> really should have been caught. Putting exclusive lwlocks and spinlocks
> in critical paths isn't a subtle, easy to miss, issue.

As one of the people that looked at the patch before it went in,
perhaps I can shed some light. I focused exclusively on the
correctness of the core mechanism. It simply didn't occur to me to
check for problems of the type you describe, that affect the system
even when the feature is disabled. I didn't check because Kevin is a
committer, that I assumed wouldn't have made such an error. Also, time
was limited.

--
Peter Geoghegan