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.


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:


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

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to