> Dan Ports wrote:
> On Wed, Jun 08, 2011 at 05:48:26PM -0500, Kevin Grittner wrote:
>> (1) Pass snapshot in to some predicate.c functions. The particular
>> functions have yet to be determined, but certainly any which
>> acquire predicate locks, and probably all which are guarded by the
>> SkipSerialization() macro. Skip processing for non-MVCC snapshots.
>> The goal here is to reduce false positive serialization failures
>> and avoid confusion about locks showing in the pg_locks view which
>> are hard to explain.
>
> I assume you've already started on this one; let me know if you
> have a patch I should take a look at or hit any snags.
A patch is attached which just covers the predicate lock acquisition,
where a snapshot is available without too much pain. There are two
functions which acquire predicate locks where a snapshot was not
readily available: _bt_search() and _bt_get_endpoint(). Not only was
it not clear how to get a snapshot in, it was not entirely clear from
reading the code that we need to acquire predicate locks here. Now,
I suspect that we probably do, because I spent many long hours
stepping through gdb to pick the spots where they are, but that was
about a year ago and my memory of the details has faded.
FWIW, this patch not only passes the usual battery of regression
tests that I run, but the test which was showing REINDEX acquiring
predicate locks on the heap runs without that happening.
I'm posting this because I think it's the most important area to
cover, and in a pinch might satisfy people for 9.1; but more
importantly to give me a good place to step back to in case I muck
things up moving forward from this point.
>> (2) Check on heap truncation from vacuum. On the face of it this
>> seems unlikely to be a problem since we make every effort to clean
>> up predicate locks as soon as there is no transaction which can
>> update what a transaction has read, but it merits a re-check. Once
>> confirmed, add a note to lazy_truncate_heap about why it's not an
>> issue.
>
> I assume you are worried here that there may be SIREAD locks
> remaining on truncated pages/tuples, and these would cause false
> positives if those pages are reused?
>
> I don't believe this can happen, because a vacuum will only delete
> a formerly-visible dead tuple if its xmax is earlier than
> OldestXmin. We remove all SIREAD locks on anything older than
> GlobalSxactXmin, which won't be less than OldestXmin.
That was my first thought, too. But, the question being raised, I
thought a quick double-check that there weren't any corner cases
where this could occur was reasonable.
>> (4) Add a comment to the docs about how querying tuples by TID
>> doesn't lock not-found "gaps" the way an indexed access would.
>
> I can take care of this one and some other README-SSI changes.
OK, I'll stay away from any doc items for now. Those are all yours
until we agree otherwise. :-) I really don't think I'm going to get
past the snapshot guard issue tonight. :-/
-Kevin