Re: [COMMITTERS] pgsql: Do all-visible handling in lazy_vacuum_page() outside its critic - Mailing list pgsql-hackers

From Jeff Davis
Subject Re: [COMMITTERS] pgsql: Do all-visible handling in lazy_vacuum_page() outside its critic
Date
Msg-id 1403772579.9081.140.camel@jeff-desktop
Whole thread Raw
In response to Re: [COMMITTERS] pgsql: Do all-visible handling in lazy_vacuum_page() outside its critic  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
On Tue, 2014-06-24 at 00:27 +0200, Andres Freund wrote:
> > Can you explain? It looks like the master will still see the bit right
> > away after the HEAP2_CLEAN record, but there is nothing in the
> > HEAP2_CLEAN record to tell the standby to set all-visible. The standby
> > has to wait until the HEAP2_VISIBLE record comes along.
>
> Nobody can see the page in that state on the master - it'll be locked
> exclusively. But that's not really the point.
>
> What could happen with 2a8e1ac5's coding is that log_heap_clean()'s
> XLogInsert() took a full page image which then contained the all visible
> bit because the PageSetAllVisible() was done *before* it. The
> standby would have replayed the HEAP2_CLEAN including the FPI and then
> unlocked the page. At that point the heap page's all visible will be set,
> but the vm bit wouldn't.
> With the new coding (ecac0e2b9) the HEAP2_CLEAN cannot log the all
> visible bit because it's not set yet. Only the HEAP2_VISIBLE will set
> it on both the heap and the vm. Makes sense?

Yes, it makes much more sense now, thank you.

> > With checksums, Simon and I tried to create some new abstractions, like
> > MarkBufferDirtyHint, and document them in transam/README. I hope that
> > gives us some greater confidence in the checksums code[1], because we
> > can see a lot of the tricky aspects in one place, along with the rules
> > that callers should follow.
>
> Hm. I don't think the current callsites dealing with this are easily
> amenable to something like that.

I haven't thought through the details, but I think that setting
PD_ALL_VISIBLE is too tricky of an operation to have different callers
doing different things.

It seems like there should be a way for all callers (there are only a
few) to set PD_ALL_VISIBLE and the VM bit the same way.

> > A quick look at the comment above the function tells us that
> > MarkBufferDirtyHint won't work for the all-visible bit, but perhaps an
> > adapted version could work. If so, we could separate the all-visible bit
> > from the HEAP2_CLEAN action, and it would look a lot more like setting a
> > tuple hint.
>
> Huh? HEAP2_CLEAN doesn't set all visible? The only reason we're now
> doing a HEAP2_VISIBLE there is that removing the tuples in the _CLEAN
> step increases the chance of it the page now being all visible? It's
> separate steps.

When setting all-visible was part of the same critical section doing the
logging for HEAP2_CLEAN, it seemed to be a part of that action. But
you're right: now it's more separate.

Regards,
    Jeff Davis




pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: Allowing join removals for more join types
Next
From: Vik Fearing
Date:
Subject: Re: idle_in_transaction_timeout