Re: Row-level Locks & SERIALIZABLE transactions, postgres vs. Oracle

On May 19, 2010, at 2:15 , Florian Pflug wrote:
> On May 17, 2010, at 3:30 , Robert Haas wrote:
>> On Sun, May 16, 2010 at 9:07 PM, Florian Pflug <fgp@phlo.org> wrote:
>>> On May 14, 2010, at 22:54 , Robert Haas wrote:
>>>> On Thu, May 13, 2010 at 5:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>>> Florian Pflug <fgp@phlo.org> writes:
>>>>>> All in all, I believe that SHARE and UPDATE row-level locks should be
>>>>>> changed to cause concurrent UPDATEs to fail with a serialization
>>>>>> error.
>>>>>
>>>>> I don't see an argument for doing that for FOR SHARE locks, and it
>>>>> already happens for FOR UPDATE (at least if the row actually gets
>>>>> updated).  AFAICS this proposal mainly breaks things, in pursuit of
>>>>> an unnecessary and probably-impossible-anyway goal of making FK locking
>>>>> work with only user-level snapshots.
>>>>
>>>> After giving this considerable thought and testing the behavior at
>>>> some length, I think the OP has it right.  One thing I sometimes need
>>>> to do is denormalize a copy of a field, e.g.
>>>>
>>>> <snipped example>
>>>
>>> I've whipped up a quick and still rather dirty patch that implements the behavior I proposed, at least for the case
ofconflicts between FOR UPDATE locks and updates. With the patch, any attempt to UPDATE or FOR UPDATE lock a row that
hasconcurrently been FOR UPDATE locked will cause a serialization error. (The same for an actually updated row of
course,but that happened before too). 
>>>
>>> While this part of the patch was fairly straight forward, make FOR SHARE conflict too seems to be much harder. The
assumptionthat a lock becomes irrelevant after the transaction(s) that held it completely is built deeply into the
multixact machinery that powers SHARE locks. That machinery therefore assumes that once all members of a multi xact
havecompleted the multi xact is dead also. But my proposal depends on a SERIALIZABLE transaction being able to find if
anyof the lockers of a row are invisible under it's snapshot - for which it'd need any multi xact containing invisible
xidsto outlive its snapshot. 
>>
>> Thanks for putting this together. I suggest adding it to the open
>> CommitFest - even if we decide to go forward with this, I don't
>> imagine anyone is going to be excited about changing it during beta.
>>
>> https://commitfest.postgresql.org/action/commitfest_view/open
>
>
> Will do. Thanks for the link.
>
> Here is an updated version that works for SHARE locks too.

Forgetting to run "make check" before sending a patch is bad, as I just proved :-(

For the archives' and the commitfest app's sake, here is a version that actually passes the regression tests.

To make up for it, I also did some testing with a custom pgbench script & schema and proved the effectiveness of this
patch.I ran this with "pgbench  -s 10 -j 10 -c 10 -t 1000 -n -f fkbench.pgbench" on both HEAD and HEAD+patch. The
formererrors out quickly with "database inconsistent" while the later completes the pgbench run without errors.  

The patch still needs more work, at least on the comments & documentation side of things, but I'm going to let this
restnow while we're in beta. 

Patch, pgbench script and schema attached.


best regards,
Florian Pflug