On 2013-09-23 14:41:16 +0300, Heikki Linnakangas wrote:
> 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)
What about the attached patches (one for 9.3 and master, the other for
9.2)? I've tested that I can trigger the assert before and not after by
inserting faults...
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services