Re: "page is not marked all-visible" warning in regression tests

On Thursday, June 07, 2012 03:20:50 PM Robert Haas wrote:
> On Wed, Jun 6, 2012 at 3:07 PM, Andres Freund <andres(at)2ndquadrant(dot)com>
wrote:
> > Hm. For a short while I thought there would be an issue with heap_delete
> > and IOS because the deleting transaction can commit without any barriers
> > happening on the IOS side. But that only seems to be possible with non
> > MVCC snapshots which are currently not allowed with index only scans.
> Well, one, commits are irrelevant; the page ceases to be all-visible
> as soon as the delete happens.
Its not irrelevant for the code as it stands if non-mvcc snapshots were
allowed. Unless I miss something, even disregarding memory ordering issues,
there is no interlocking providing protection against the index doing the
visibilitymap_test() and some concurrent backend doing a heap_delete + commit
directly after that. If a SnapshotNow were used this could result in different
results between a IOS and a normal indexscan because the normal indexscan
would lock the heap page before doing the visibility testing and thus would
see the new visibility information. But then a SnapshotNow wouldn't be used if
that were a problem...
For normal snapshots its not a problem because with regards to the snapshot
everything on the page is still visible as I think that can only happen for
deletions and HOT updates because the index page would need to get locked
otherwise. Deletions aren't visible yet and HOT is irrelevant for the IOS by
definition.

> And two, you can't do a delete or a commit without a memory barrier - every
> LWLockAcquire() or LWLockRelease() is a full barrier, so any operation that
> requires a buffer content lock is both preceded and followed by a full
> barrier.
The memory barrier on the deleting side is irrelevant if there is no memory
barrier on the reading side.

> Proposed patch attached. This adds some more comments in various
> places, and implements your suggestion of retesting the visibility-map
> bit when we detect a possible mismatch with the page-level bit.
Thanks, will look at it in a bit.

Andres

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services