Re: SSI bug?

On Tue, Feb 22, 2011 at 10:51:05AM -0600, Kevin Grittner wrote:
> Dan Ports <drkp(at)csail(dot)mit(dot)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/