On 01/13/2014 10:26 PM, Andres Freund wrote:
> On 2014-01-13 15:19:06 -0500, Tom Lane wrote:
>> I concur that the right fix requires a
>> new operating mode for XLogReadBufferExtended, perhaps RBM_NORMAL_ZERO_OK.
>> I think the spec for this should be that if the page doesn't exist or
>> contains zeroes, we return InvalidBuffer without logging the page number
>> as invalid. The doesn't-exist case is justified by the expectation that
>> there will be a later RBM_NORMAL call for a larger page number, which will
>> result in a suitable complaint if the page range isn't there.
I think it's more natural to return the empty page to the caller, rather
than InvalidBuffer.
> That's a sensible way to go, yes. But I wonder if we wouldn't end up
> with less complicated code if we added a variant of ReadBuffer that only
> returns a buffer from cache if already present in s_b.
> I started to prototype something like RBM_NORMAL_ZERO_OK shortly after
> Heikki's message and it seems to quickly turn a bit ugly because the
> surrounding code isn't really ready to deal with not returning a
> buffer. And for the purpose of that redo routine, that'd actually be
> better.
If it's trivial to add such a mode to buffer cache, then sure, let's do
that by all means. But I seriously doubt it's really simple enough to be
back-patchable.
With RBM_NORMAL_ZERO_OK, AFAICS we're talking about a tiny patch to
XLogReadBufferExtended. bufmgr.c doesn't need to do anything about the
new mode, as it's XLogReadBuffer that does the the check for zero pages.
Per attached patch (for demonstration purposes only, you also need to
add the new mode to the header file and adjust comments).
- Heikki