Re: WAL format and API changes (9.5)

On Thu, Aug 14, 2014 at 2:05 AM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:
> Here's a full version of this refactoring patch, all the rmgr's have now
> been updated to use XLogReplayBuffer(). I think this is a worthwhile change
> on its own, even if we drop the ball on the rest of the WAL format patch,
> because it makes the redo-routines more readable. I propose to commit this
> as soon as someone has reviewed it, and we agree on a for what's currently
> called XLogReplayBuffer(). I have tested this with my page-image comparison
> tool.

I have as well passed this patch through the page comparison tool and
found no problems, with both regression and isolation tests. I also
had a look at the redo routines that are changed and actually found
nothing. Now, what this patch does is not much complicated, it adds a
couple of status flags returned by XLogReplayBuffer. Then, roughly,
when BLK_NEEDS_REDO is returned the previous restore actions are done.
This has the merit to put the LSN check on current page to determine
if a page needs to be replayed inside XLogReplayBuffer.

A couple of minor comments though:
1) Why changing that from ERROR to PANIC?       /* Caller specified a bogus block_index */
-       elog(ERROR, "failed to restore block_index %d", block_index);
+       elog(PANIC, "failed to restore block_index %d", block_index);
2) There are some whitespaces, like here:
+               {
+                       destPage = NULL;        /* don't do any page updates */               }

Regards,
-- 
Michael