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

On Thursday, June 07, 2012 06:08:34 PM Robert Haas wrote:
> On Thu, Jun 7, 2012 at 11:04 AM, Andres Freund <andres@2ndquadrant.com> 
wrote:
> > On Thursday, June 07, 2012 04:27:32 PM Robert Haas wrote:
> >> On Thu, Jun 7, 2012 at 9:41 AM, Andres Freund <andres@2ndquadrant.com>
> > 
> > wrote:
> >> >> 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.
> > The visibilitymap_clear/PageClearAllVisible in heap_multi_insert should
> > be moved into the critical section, shouldn't it?
> 
> Yes, it should.  I was thinking maybe we could go the other way and
> have heap_insert do it before starting the critical section, but
> that's no good: clearing the visibility map bit is part of the
> critical data change, and we can't do it and then forget to WAL-log
> it.
You could do a visibilitymap_pin outside, but I don't really see the point as 
the page is already locked. There might be some slight benefit in doing so in 
multi_insert but that would be more complicated. And of doubtful benefit.

> Updated patch attached.
Looks good.

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