heapgetpage() and ->takenDuringRecovery - Mailing list pgsql-hackers

From Andres Freund
Subject heapgetpage() and ->takenDuringRecovery
Date
Msg-id 20140302133905.GC18320@alap3.anarazel.de
Whole thread Raw
Responses Re: heapgetpage() and ->takenDuringRecovery  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
Hi,

I am currently playing around with Robert's suggestion to get rid of
changeset extraction's reusage of SnapshotData fields (basically that
xip contains committed, not uncommited transactions) by using NodeTag
similar to many other (families of) structs.

While reading around which references to SnapshotData's members exist, I
once more came about the following tidbit in heapgetpage():/* * If the all-visible flag indicates that all tuples on
thepage are * visible to everyone, we can skip the per-tuple visibility tests. * * Note: In hot standby, a tuple that's
alreadyvisible to all * transactions in the master might still be invisible to a read-only * transaction in the
standby.We partly handle this problem by tracking * the minimum xmin of visible tuples as the cut-off XID while marking
a* page all-visible on master and WAL log that along with the visibility * map SET operation. In hot standby, we wait
for(or abort) all * transactions that can potentially may not see one or more tuples on the * page. That's how
index-onlyscans work fine in hot standby. A crucial * difference between index-only scans and heap scans is that the *
index-onlyscan completely relies on the visibility map where as heap * scan looks at the page-level PD_ALL_VISIBLE
flag.We are not sure if * the page-level flag can be trusted in the same way, because it might * get propagated somehow
withoutbeing explicitly WAL-logged, e.g. via a * full page write. Until we can prove that beyond doubt, let's check
each* tuple for visibility the hard way. */all_visible = PageIsAllVisible(dp) && !snapshot->takenDuringRecovery;
 

I don't think this is neccessary >= 9.2. The are two only "interestings" place
where PD_ALL_VISIBLE is set:
a) lazy_vacuum_page() where a xl_heap_clean is logged *before*  PD_ALL_VISIBLE/the vm is touched and that causes
recovery conflicts. The heap page is locked for cleanup at that point. As the  logging of xl_heap_clean sets the page's
LSNthere's no way the page  can appear on the standby too early.
 
b) empty pages in lazy_scan_heap(). If they always were empty, there's  no need for conflicts. The only other way I can
seeto end up there  is a previous heap_page_prune() that repaired fragmentation. But that  logs a WAL record with
conflictinformation.
 

So, we could just remove this?

Greetings,

Andres Freund

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



pgsql-hackers by date:

Previous
From: Fabien COELHO
Date:
Subject: Re: gaussian distribution pgbench
Next
From: Magnus Hagander
Date:
Subject: Re: Securing "make check" (CVE-2014-0067)