On 03.10.2013 01:05, Kevin Grittner wrote:
> Andres Freund<andres@2ndquadrant.com> wrote:
>> On 2013-10-01 07:41:46 -0700, Kevin Grittner wrote:
>>> Andres Freund<andres@2ndquadrant.com> wrote:
>>>
>>>> A better solution probably is to promote tuple-level locks if
>>>> they exist to a relation level one upon freezing I guess?
>>>
>>> It would be sufficient to promote the tuple lock to a page lock.
>>> It would be pretty easy to add a function to predicate.c which
>>> would accept a Relation and HeapTuple, check for a predicate lock
>>> for the tuple, and add a page lock if found (which will
>>> automatically clear the tuple lock). This new function would be
>>> called when a tuple was chosen for freezing. Since freezing always
>>> causes WAL-logging and disk I/O, the cost of a couple hash table
>>> operations should not be noticeable.
>>
>> Yea, not sure why I was thinking of table level locks.
>>
>>> This seems like a bug fix which should be back-patched to 9.1, yes?
>>
>> Yes.
>
> Patch attached, including new isolation test based on Heikki's
> example. This patch does change the signature of
> heap_freeze_tuple(). If anyone thinks there is risk that external
> code may be calling this, I could keep the old function with its
> old behavior (including the bug) and add a new function name with
> the new parameters added -- the old function could call the new one
> with NULL for the last two parameters. I'm not sure whether that
> is better than breaking a compile of code which uses the old
> signature, which would force a choice about what to do.
>
> Will give it a couple days for feedback before pushing.
IMHO it would be better to remove xmin from the lock key, and vacuum
away the old predicate locks when the corresponding tuple is vacuumed.
The xmin field is only required to handle the case that a tuple is
vacuumed, and a new unrelated tuple is inserted to the same slot.
Removing the lock when the tuple is removed fixes that.
In fact, I cannot even come up with a situation where you would have a
problem if we just removed xmin from the key, even if we didn't vacuum
away old locks. I don't think the old lock can conflict with anything
that would see the new tuple that gets inserted in the same slot. I have
a feeling that you could probably prove that if you stare long enough at
the diagram of a dangerous structure and the properties required for a
conflict.
- Heikki