On Tue, Dec 24, 2013 at 4:09 AM, Andres Freund <and...@2ndquadrant.com> wrote: > I don't really see the lack of review as being crucial at this point. At > least I have quite some doubts about the approach you've chosen and I > have voiced it - so have others.
Apparently you haven't been keeping up with this thread. The approach that Heikki outlined with his POC patch was demonstrated to deadlock in an unprincipled manner - it took me a relatively long time to figure this out because I didn't try a simple enough test-case. There is every reason to think that alternative promise tuple approaches would behave similarly, short of some very invasive, radical changes to how we wait on XID share locks that I really don't think are going to fly. That's why I chose this approach: at no point did anyone have a plausible alternative that didn't have similar problems, and I personally saw no alternative. It wasn't really a choice at all. In hindsight I should have known better than to think that people would be willing to defer discussion of a more acceptable value locking implementation to consider the interactions between the different subsystems, which I felt were critical and warranted up-front discussion, a belief which has now been borne out. Lesson learned. It's a pity that that's the way things are, because that discussion could have been really useful, and saved us all some time. > I don't think there's too much reviewers can do before you've provided a > POC implementation of real value locking. I don't see what is functionally insufficient about the value locking that you've already seen. I'm currently working towards extended the buffer locking to use a heavyweight lock held only for an instant, but potentially across multiple operations, although of course only when upserting occurs so as to not regress regular insertion. If you're still of the opinion that it is necessary to hold value locks of some form on earlier unique indexes, as you wait maybe for hours on some conflicting xid, then I still disagree with you for reasons recently re-stated [1]. You never stated a reason why you thought it was necessary. If you have one now, please share it. Note that I release all value locks before row locking too, which is necessary because to do any less will cause unprincipled deadlocking, as we've seen. Other than that, I have no idea what your continued objection to my design would be once the buffer level exclusive locks are replaced with page level heavyweight locks across complex (though brief) operations (I guess you might not like the visibility stuff or the syntax, but that isn't what you're talking about here). More granular value locking might help boost performance, but maybe not even by much, since we're only locking a single leaf page per unique index against insertion, and only for an instant. I see no reason to make the coarser-than-necessary granularity of the value locking a blocker. Predicate locks on btree leaf pages acquired by SSI are also coarser than strictly necessary. [1] http://www.postgresql.org/message-id/cam3swzsodumg4899tjc09r2uortyhb0vl9aasc1fz7aw4gs...@mail.gmail.com -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
