Re: pgsql: Do all-visible handling in lazy_vacuum_page() outside its critic

On 2014-06-26 10:39:01 +0300, Heikki Linnakangas wrote:
> On 06/24/2014 01:27 AM, Andres Freund wrote:
> >Does your change still make
> >sense to you and do you see problem with the current state (as of ecac0e2b)?
>
> Hmm, in the current state, it's again possible that the full-page image
> doesn't contain the all-visible flag, even though the page in the buffer
> does. I guess that's OK, because replaying XLOG_HEAP2_VISIBLE always sets
> the flag. My page-comparison tool will complain, so it would be nice to not
> do that, but it's a false alarm.

I don't see where that difference should come from? On both the primary
and standby neither the page nor the vm will have all-visible bit set
after the HEAP2_CLEAN. Both will have it set after the
HEAP2_VISIBLE.
The only chance for a disparity that I see is when crashing after
PageSetAllVisible(), but before the XLogInsert(). That's fairly harmless
afaics and I don't think your tool would pick that up?

I think we could fix it by doing something like

if (!PageIsAllVisible(page) && heap_page_is_all_visible())
{
     visibilitymap_pin(onerel, blkno, vmbuffer)

     START_CRIT_SECTION();
     PageSetAllVisible(page);
     if (!visibilitymap_test(onerel, blkno, vmbuffer))
     {
        visibilitymap_set(onerel, blkno, buffer, InvalidXLogRecPtr, *vmbuffer,
                          visibility_cutoff_xid);
     }
     END_CRIT_SECTION();
}

If we care.

Greetings,

Andres Freund

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