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