Re: ReadRecentBuffer() is broken for local buffer

On Mon, Jul 25, 2022 at 2:22 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:

> if (BufferIsLocal(recent_buffer))
> {
> - bufHdr = GetBufferDescriptor(-recent_buffer - 1);
> + bufHdr = GetLocalBufferDescriptor(-recent_buffer - 1);

Aha, we're using the wrong buffer descriptors here. Currently this
function is only called in XLogReadBufferExtended(), so the branch for
local buffer cannot be reached. Maybe that's why it is not identified
until now.

> The code after that looks suspicious, too. It increases the usage count
> even if the buffer was already pinned. That's different from what it
> does for a shared buffer, and different from LocalBufferAlloc(). That's
> pretty harmless, just causes the usage count to be bumped more
> frequently, but I don't think it was intentional. The ordering of
> bumping the usage count, the local ref count, and registration in the
> resource owner are different too. As far as I can see, that makes no
> difference, but I think we should keep this code as close as possible to
> similar code used elsewhere, unless there's a particular reason to differ.

Agree. Maybe we can wrap the codes in an inline function or macro and
call that in both LocalBufferAlloc and here.

Thanks
Richard