Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL

Fujii Masao wrote:
> But in the current (v8.4 or before) behavior, recovery ends normally
> when an invalid record is found in an archived WAL file. Otherwise,
> the server would never be able to start normal processing when there
> is a corrupted archived file for some reasons. So, that invalid record
> should not be treated as a PANIC if the server is not in standby mode
> or the trigger file has been created. Thought?

Hmm, true, this changes behavior over previous releases. I tend to think
that it's always an error if there's a corrupt file in the archive,
though, and PANIC is appropriate. If the administrator wants to start up
the database anyway, he can remove the corrupt file from the archive and
place it directly in pg_xlog instead.

> When I tested the patch, the following PANIC error was thrown in the
> normal archive recovery. This seems to derive from the above change.
> The detail error sequence:
> 1. In ReadRecord(), emode was set to PANIC after 00000001000000000000000B
> was read.
> 2. 00000001000000000000000C including the contrecord tried to be read
> by using the emode (= PANIC). But since 00000001000000000000000C did
> not exist, PANIC error was thrown.
>
> -----------------
> LOG: restored log file "00000001000000000000000B" from archive
> cp: cannot stat `../data.arh/00000001000000000000000C': No such file
> or directory
> PANIC: could not open file "pg_xlog/00000001000000000000000C" (log
> file 0, segment 12): No such file or directory
> LOG: startup process (PID 17204) was terminated by signal 6: Aborted
> LOG: terminating any other active server processes
> -----------------

Thanks. That's easily fixable (applies over the previous patch):

--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3773,7 +3773,7 @@ retry:
pagelsn.xrecoff = 0;
}
/* Wait for the next page to become available */
- if (!XLogPageRead(&pagelsn, emode, false, false))
+ if (!XLogPageRead(&pagelsn, emode_arg, false, false))
return NULL;

/* Check that the continuation record looks valid */

Perhaps the emode/emode_arg convention is a bit hard to read.

I'll go through the patch myself once more, and commit later today or
tomorrow if now new issues crop up.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com