Re: change in LOCK behavior

On Thu, Oct 11, 2012 at 2:48 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> Hmm, so now the patch author thinks his patch is not just broken with
> respect to lock waits, but in all cases? Surely the above race
> condition is obvious, now and before. Why is it suddenly unacceptable?
> (If you believe that, why on earth did you commit?)
>
> Whenever you take a snapshot things can change before you start using
> it. And as a result all previous releases of Postgres suffer this
> problem.

I agree with this. I think that the reversion that Tom is pushing for
is fixing a very special case of a more general problem. Now, it
might be a sufficiently important special-case that we ought to go and
do the revert, but if your argument is that this is an unsatisfying
band-aid, I couldn't agree more. It appears to me that this
discussion is exposing the fact that we really haven't given much
thought to when snapshots ought to be taken or to arranging the order
of operations so that they are taken as late as is safely possible.

> Ergo, we should defer taking the snapshot until the very last
> point when we need to use it. Why is that *not* being suggested here?

Well, that's actually not safe either, at least if taken literally.
For example, you can't do part of a sequential scan without a snapshot
just because all the tuples are frozen, and then take a snapshot when
you hit a unfrozen xmin/xmax. Both you and I previously came up with
that idea independently, and it would save a lot of cycles, but it's
unsafe. Some other backend can update a tuple after you look at it,
and then commit. When you get to the new tuple, you'll already have
returned the old tuple, and your new snapshot - since it's taken after
the commit - will also see the new tuple.

So we have to take the snapshot before you begin execution, but it
seems that to avoid surprising behavior we also have to take it after
acquiring locks. And it looks like locking is intertwined with a
bunch of other parse analysis tasks that might require a snapshot to
have been taken first. Whee.

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