On Tue, Feb 22, 2011 at 10:51:05AM -0600, Kevin Grittner wrote: > Dan Ports <d...@csail.mit.edu> wrote: > > > It looks like CheckTargetForConflictsIn is making the assumption > > that the backend-local lock table is accurate, which was probably > > even true at the time it was written. > > I remember we decided that it could only be false in certain ways > which allowed us to use it as a "lossy" first-cut test in a couple > places. I doubt that we can count on any of that any longer, and > should drop those heuristics.
Hmm. The theory was before that the local lock table would only have false negatives, i.e. if it says we hold a lock then we really do. That makes it a useful heuristic because we can bail out quickly if we're trying to re-acquire a lock we already hold. It seems worthwhile to try to preserve that. This property holds as long as the only time one backend edits another's lock list is to *add* a new lock, not remove an existing one. Fortunately, this is true most of the time (at a glance, it seems to be the case for the new tuple update code). There are two exceptions; I think they're both OK but we need to be careful here. - if we're forced to promote an index page lock's granularity to avoid running out of shared memory, we remove the fine-grained one. This is fine as long as we relax our expectations to "if the local lock table says we hold a lock, then we hold that lock or one that covers it", which is acceptable for current users of the table. - if we combine two index pages, we remove the lock entry for the page being deleted. I think that's OK because the page is being removed so we will not make any efforts to lock it. > Yeah, as far as a I can recall the only divergence was in *page* > level entries for *indexes* until this latest patch. We now have > *tuple* level entries for *heap* relations, too. *nod*. I'm slightly concerned about the impact of that on granularity promotion -- the new locks created by heap updates won't get counted toward the lock promotion thresholds. That's not a fatal problem of anything, but it could make granularity promotion less effective at conserving lock entries. > > The solution is only slightly more complicated than just removing > > the assertion. > > That's certainly true for that one spot, but we need an overall > review of where we might be trying to use LocalPredicateLockHash for > "first cut" tests as an optimization. Yes, I'd planned to go through the references to LocalPredicateLockHash to make sure none of them were making any unwarranted assumptions about the results. Fortunately, there are not too many of them... Dan -- Dan R. K. Ports MIT CSAIL http://drkp.net/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
