Re: what to revert - Mailing list pgsql-hackers

On Wed, May 4, 2016 at 12:42 PM, Andres Freund <andres@anarazel.de> wrote:
> On 2016-05-04 13:35:02 -0300, Alvaro Herrera wrote:
>> Honestly, I don't see any strong ground in which to base a revert threat
>> for this feature.
>
> It's datastructures are badly designed. But releasing it there's no
> pressure to fix that.  If this were an intrinsic cost - ok. But it's
> not.

I don't want to rule out the possibility that you are right, because
you're frequently right about lots of things.  However, you haven't
provided much detail.  AFAIK, the closest you've come to articulating
a case for reverting this patch is to say that the tax ought to be
paid by the write-side, rather than the read-side.   I don't know
exactly what that means, though.  Snapshots are not stored in shared
memory; writers can't iterate through all snapshots and whack the ones
that are problematic - and even if they could, they can't know what
tuples will be read in the future, which determines whether or not
there is an actual problem.  We could dispense with a lot of this
machinery if we simply killed off transactions or sessions that stuck
around too long, but the whole point of this feature is to avoid
having to do that when it isn't really necessary.  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.  So I would like to hear more detail about why you
think that the data structures are badly designed, and how they could
be designed differently without changing what the patch does (which
amounts to wishing that Kevin had implemented a different feature than
the one you think he should have implemented).

>> It doesn't scale as well as we would like in the case
>> where a high-level is fully stressed with a read-only load -- okay.  But
>> that's unlikely to be a case where this feature is put to work.
>
> It'll be just the same in a read mostly workload, which is part of the
> case for this feature.

So what?  SSI doesn't scale either, and nobody's saying we have to rip
it out.  It's still useful to people.  pgbench is not the world.

>> So I think accepting the promise that this feature would be improved
>> in a future release to better support that case is good enough.
>
> I've not heard any such promise.

The question Alvaro is raising isn't whether such a promise has been
made but whether it would suffice.  In fairness, such promises are not
enforceable.  Kevin can promise to work on this and then be run over
by a rogue Mr. Softee truck tomorrow, and the work won't get done; or
he can go to work for some non-PostgreSQL-supporting company and
vanish.  I hope neither of those things happens, but if we accept a
promise to improve it, it's going to be contingent on certain things
happening or not happening which are beyond the ability of the
PostgreSQL community to effect or forestall.  (FWIW, I believe I can
safely say that EnterpriseDB will support his continued work on this
feature for as long as the community has concerns about it and Kevin
works for EnterpriseDB.)

But personally, I generally agree with Alvaro's analysis.  If this
patch is slow when turned on, and you don't like that, don't use it.
If you need to use it and want it to scale better, then you can write
a patch to make it do that.  Nobody is more qualified than you.  I
think it's a show-stopper if the patch regresses performance more than
trivially when turned off, but we need fresh test results before
reaching a conclusion about that.  I also think it's a show-stopper if
you can hold out specific design issues which we can generally agree
are signs of serious flaws in Kevin's thinking.  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.
If somebody had even hinted that such a problem might exist, Kevin
probably would have fixed it before commit, but nobody did.  As soon
as it was raised, Kevin started working on it.  That's about all you
can ask, I think.

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