Re: BUG #8686: Standby could not restart.

On 12/19/2013 04:57 AM, katsumata.tomonari@po.ntts.co.jp wrote:
> At first, I doubted the recovery state reached "consistent" before redo
> starts.
> And then I checked pg_control and related WAL.
> The WAL sequence is like below.
>
>
> WAL--(a)--(b)--(c)--(d)--(e)-->
> ================================================
> (a) Latest checkpoint's REDO location
> 1/783B230
>
>
> (b) hot_update
> 1/7842010
>
>
> (c) truncate
> 1/8E7E5C8
>
>
> (d) Latest checkpoint location
> 1/8E7F0B0
>
>
> (e) Minimum recovery ending location
> 1/8E7F110
> ================================================
>
>
>>From these things, I found it has happened with this scenario.
> ----------
> (1) standby starting
> (2) seeking checkpoint location 1/8E7F0B0 because backup_label is not
> absecnt
> (3) reachedConsistency is set to true at 1/8E7F110 in
> CheckRecoveryConsistent
> (4) redo start from 1/783B230
> (5) PANIC at 1/7842010 because reachedConsistency has set already and
> operating against a block which will be truncated at (c).
> ----------
>
> At step(2), EndRecPtr is set to 1/8E7F110(next to 1/8E7F0B0),
> so reachedConsistency is set to true at step(3).

Yep. Thanks for a good explanation.

> I think it's not need to increase EndRecPtr while seeking checkpoint
> location.
> I tried to revise it and this worked fine.

Hmm. There's this comment in StartupXLOG, after reading the checkpoint
record, but before reading the first record at REDO point:


>         /*
>          * Initialize shared replayEndRecPtr, lastReplayedEndRecPtr, and
>          * recoveryLastXTime.
>          *
>          * This is slightly confusing if we're starting from an online
>          * checkpoint; we've just read and replayed the checkpoint record, but
>          * we're going to start replay from its redo pointer, which precedes
>          * the location of the checkpoint record itself. So even though the
>          * last record we've replayed is indeed ReadRecPtr, we haven't
>          * replayed all the preceding records yet. That's OK for the current
>          * use of these variables.
>          */
>         SpinLockAcquire(&xlogctl->info_lck);
>         xlogctl->replayEndRecPtr = ReadRecPtr;
>         xlogctl->lastReplayedEndRecPtr = EndRecPtr;
>         xlogctl->recoveryLastXTime = 0;
>         xlogctl->currentChunkStartTime = 0;
>         xlogctl->recoveryPause = false;
>         SpinLockRelease(&xlogctl->info_lck);

I think we need to fix that confusion. Your patch will do it by not
setting EndRecPtr yet; that fixes the bug, but leaves those variables in
a slightly strange state; I'm not sure what EndRecPtr points to in that
case (0 ?), but ReadRecPtr would be set I guess.

Perhaps we should reset replayEndRecPtr and lastReplayedEndRecPtr to the
REDO point here, instead of ReadRecPtr/EndRecPtr.

- Heikki