Re: WAL format and API changes (9.5)

On 08/19/2014 11:07 AM, Michael Paquier wrote:
> Patch 1 looks good. The main difference with the v1 submitted a couple of
> days back is that the global variable approach is replaced with additional
> arguments for the LSN position and record pointer in XLogReplayBuffer. I
> have as well run a couple of tests with the page comparison tool, done some
> tests based on installcheck-world with a slave replaying WAL behind, and
> found no issues with it.

Thanks!

> Perhaps we could consider pushing it to facilitate the next work? Even if
> the second patch is dropped it is still a win IMO to have backup block
> replay managed within a single function (being it named XLogReplayBuffer in
> latest patch), and having it return a status flag.

Yeah, that was my plan.

Regarding the name, the following have been suggested so far:

XLogReplayBuffer
XLogRestoreBuffer
XLogRecoverBuffer
XLogReadBufferForReplay
ReadBufferForXLogReplay

One more idea:

XLogRedoBuffer (would be like three first options above, but would match 
that we call the functions that call this "redo" functions)

I think XLogReadBufferForReplay is the most descriptive. Andres and 
Alvaro both suggested it - independently I believe - so that seems to 
come out naturally. But maybe make it XLogReadBufferForRedo, since we 
call the redo functions redo functions and not replay functions.

Yet another option is to just call it XLogReadBuffer, and rename the 
existing XLogReadBuffer to something else. With the 2nd patch, there are 
only a few callers of XLogReadBuffer left. But is it too deceitful if 
XLogReadBuffer doesn't merely read the page, but also sometimes replaces 
it with a full-page image? Maybe it's OK..


Barring objections or better ideas, I'm leaning towards 
XLogReadBufferForRedo.

- Heikki