Re: all_visible replay aborting due to uninitialized pages

On 06.06.2013 17:22, Robert Haas wrote:
> On Thu, May 30, 2013 at 2:29 AM, Andres Freund<andres@2ndquadrant.com>  wrote:
>>> Yeah, I think it's fine.  The patch also looks fine, although I think
>>> the comments could use a bit of tidying.  I guess we need to
>>> back-patch this all the way back to 8.4?  It will require some
>>> adjustments for the older branches.
>>
>> I think 9.2 is actually far enough and it should apply there. Before
>> that we only logged the unsetting of all_visible via
>> heap_(inset|update|delete)'s wal records not the setting as far as I can
>> tell. So I don't immediately see a danger<  9.2.
>
> OK.  I have committed this.  For 9.2, I had to backport
> log_newpage_buffer() and use XLByteEQ rather than ==.

I'm afraid this patch was a few bricks shy of a load. The 
log_newpage_buffer() function asserts that:

>     /* We should be in a critical section. */
>     Assert(CritSectionCount > 0);

But the call in vacuumlazy.c is not inside a critical section. Also, the 
comments in log_newpage_buffer() say that the caller should mark the 
buffer dirty *before* calling log_newpage_buffer(), but in vacuumlazy.c, 
it's marked dirty afterwards. I'm not sure what consequences that might 
have, but at least it contradicts the comment.

(spotted this while working on a patch, and ran into the assertion on 
crash recovery)

- Heikki