Re: PageIsAllVisible()'s trustworthiness in Hot Standby

On Tue, Dec 4, 2012 at 8:08 AM, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com> wrote:
>
> I was looking at the following code in heapam.c:
>
> 261 /*
> 262 * If the all-visible flag indicates that all tuples on the page
> are
> 263 * visible to everyone, we can skip the per-tuple visibility tests.
> But
> 264 * not in hot standby mode. A tuple that's already visible to all
> 265 * transactions in the master might still be invisible to a
> read-only
> 266 * transaction in the standby.
> 267 */
> 268 all_visible = PageIsAllVisible(dp) &&
> !snapshot->takenDuringRecovery;
>
> Isn't the check for !snapshot->takenDuringRecovery redundant now in master
> or whenever since we added crash-safety for VM ? In fact, this comment made
> me think if we are really handling index-only scans correctly or not on the
> Hot Standby. But apparently we are by forcing conflicting transactions to
> abort before redoing VM bit set operation on the standby. The same mechanism
> should protect us against the above case. Now I concede that the entire
> magic around setting and clearing the page level all-visible bit and the VM
> bit and our ability to keep them in sync is something I don't fully
> understand, but I see that every operation that sets the page level
> PD_ALL_VISIBLE flag also sets the VM bit while holding the buffer lock and
> emits a WAL record. So AFAICS the conflict resolution logic will take care
> of the above too.

I wasn't sure whether that could be safely changed. There's a subtle
distinction here: the PD_ALL_VISIBLE bit isn't the same as the
visibility map bit. And, technically, the WAL record only fully
protects the setting of *the visibility map bit* not the
PD_ALL_VISIBLE page-level bit. The purpose of that WAL logging is to
make sure that the page-level bit is never clear while the
visibility-map bit is set; it does not guarantee that the page-level
bit can never be set without issuing a WAL record. So, for example,
it's perfectly possible for a crash on the master might leave the
page-level bit set while the VM bit is clear. Now, if that page
somehow makes its way to the standby - via a base backup or a
full-page image - before the tuples it contains are all-visible
according to the standby's xmin horizon, we've got a problem. Can
that happen? It seems unlikely, but can we prove it's not possible?
Perhaps, but I wasn't sure.

Index-only scans are safe, because they're looking at the visibility
map itself, not the page-level bit, but the analysis is a little
murkier for sequential scans.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company