Re: Hot standby queries see transient all-zeros pages

From: Noah Misch <noah(at)leadboat(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Hot standby queries see transient all-zeros pages
Date: 2024-12-14 00:38:05
Message-ID: 20241214003805.f4.nmisch@google.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Dec 13, 2024 at 05:41:15PM -0500, Andres Freund wrote:
> Afaics we didn't do anything about this issue?

postgr.es/c/e656657 fixed this. I thought this was longstanding, but it
turned out to have started on 2024-04-02.

> Hm. Leaving RBM_ZERO_AND_LOCK aside, is it actually always safe to do
> RestoreBlockImage() into a buffer that currently is pinned? Not sure if
> there's actually all that much guarantee what transient state one can read
> when reading a page concurrently to a memcpy(). I suspect it's practically
> rare to see a problem, but one could imagine an memcpy implementation that
> uses non-temporal writes, which afaict would leave you open to seeing quite
> random states when reading concurrently, as the cache coherence protocol
> doesn't protect anymore.

I wondered about that, too. I didn't dig too deep.
https://pubs.opengroup.org/onlinepubs/9799919799/functions/memcpy.html and
https://en.cppreference.com/w/cpp/string/byte/memcpy were both silent about
the topic.

> On 2024-05-12 10:16:58 -0700, Noah Misch wrote:
> > I suspect the fix is to add a ReadBufferMode specified as, "If the block is
> > already in shared_buffers, do RBM_NORMAL and exclusive-lock the buffer.
> > Otherwise, do RBM_ZERO_AND_LOCK."
>
> I think that should work. At least in the current code it looks near trivial
> to implement, although the branch differences are going to be annoying.
>
> As usual the hardest part would probably be the naming. Maybe
> RBM_ZERO_ON_MISS_LOCK? RBM_LOCK_ZERO_ON_MISS? RBM_DWIM?

It turned out RBM_ZERO_AND_LOCK long worked that way, and postgr.es/c/e656657
just had to restore that longstanding behavior. The existing comment "Don't
read from disk, caller will initialize." does allude to this (but I didn't
originally catch the subtle point).

If RBM_ZERO_AND_LOCK hadn't existed so long, I'd rename it. Perhaps it
deserves a rename anyway? Of those, I'd pick RBM_ZERO_ON_MISS_LOCK. I also
considered RBM_RECENT_OR_ZERO, borrowing a term from ReadRecentBuffer().

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2024-12-14 00:41:40 Re: Hot standby queries see transient all-zeros pages
Previous Message Heikki Linnakangas 2024-12-14 00:06:53 Re: Recovering from detoast-related catcache invalidations