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 <f...@phlo.org> wrote: >>> On May 14, 2010, at 22:54 , Robert Haas wrote: >>>> On Thu, May 13, 2010 at 5:39 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >>>>> Florian Pflug <f...@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 of conflicts between FOR UPDATE >>> locks and updates. With the patch, any attempt to UPDATE or FOR UPDATE lock >>> a row that has concurrently 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 assumption that a lock becomes >>> irrelevant after the transaction(s) that held it completely is built deeply >>> into the multi xact machinery that powers SHARE locks. That machinery >>> therefore assumes that once all members of a multi xact have completed the >>> multi xact is dead also. But my proposal depends on a SERIALIZABLE >>> transaction being able to find if any of the lockers of a row are invisible >>> under it's snapshot - for which it'd need any multi xact containing >>> invisible xids to 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 former errors 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 rest now while we're in beta. Patch, pgbench script and schema attached.
serializable_lock_consistency.patch
Description: Binary data
fkbench.init.sql
Description: Binary data
fkbench.pgbench
Description: Binary data
best regards, Florian Pflug
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
