Re: SSI freezing bug

Kevin Grittner <kgrittn(at)ymail(dot)com> wrote:
> Kevin Grittner <kgrittn(at)ymail(dot)com> wrote:
>
>> I'm strongly leaning toward the idea that a slightly tweaked
>> version of the proposed patch is appropriate for the back-branches,
>> because the fix Heikki is now suggesting seems too invasive to
>> back-patch.  I think it would make sense to apply it to master,
>> too, so that the new isolation tests can be immediately added.  We
>> can then work on the new approach for 9.4 and have the tests to
>> help confirm we are not breaking anything.  The tweak would be to
>> preserve the signature of heap_freeze_tuple(), because after the
>> more invasive fix in 9.4 the new parameters will not be needed.
>> They are only passed as non-NULL from one of the three callers, so
>> it seems best to leave those call sites alone rather than change
>> them back-and-forth.
>>
>> I will post a new patch today or tomorrow.
>
> Attached.

And here's a rough cut of what I think the alternative now
suggested by Heikki would look like.  (I've omitted the new
isolation test because that is the same as the previously posted
patch.)

Note this comment, which I think was written by Heikki back when
there was a lot more benchmarking and profiling of SSI going on:

  * Because a particular target might become obsolete, due to update to a new
  * version, before the reading transaction is obsolete, we need some way to
  * prevent errors from reuse of a tuple ID.  Rather than attempting to clean
  * up the targets as the related tuples are pruned or vacuumed, we check the
  * xmin on access.    This should be far less costly.

Based on what this patch looks like, I'm afraid he may have been
right when he wrote that.  In any event, after the exercise of
developing a first draft of searching for predicate locks to clean
up every time a tuple is pruned or vacuumed, I continue to feel
strongly that the previously-posted patch, which only takes action
when tuples are frozen by a vacuum process, is much more suitable
for backpatching.  I don't think we should switch to anything
resembling the attached without some careful benchmarking.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company