From: | Jeff Davis <pgsql(at)j-davis(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, pgsql-committers(at)postgresql(dot)org, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: pgsql: Do all-visible handling in lazy_vacuum_page() outside its critic |
Date: | 2014-06-26 08:49:39 |
Message-ID: | 1403772579.9081.140.camel@jeff-desktop |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers 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
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2014-06-26 09:38:47 | Re: pgsql: Do all-visible handling in lazy_vacuum_page() outside its critic |
Previous Message | Heikki Linnakangas | 2014-06-26 07:39:01 | Re: pgsql: Do all-visible handling in lazy_vacuum_page() outside its critic |
From | Date | Subject | |
---|---|---|---|
Next Message | Vik Fearing | 2014-06-26 08:50:04 | Re: idle_in_transaction_timeout |
Previous Message | David Rowley | 2014-06-26 08:46:30 | Re: Allowing join removals for more join types |