Re: [BUG] non archived WAL removed during production crash recovery

At Tue, 21 Apr 2020 13:57:39 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> On Tue, Apr 21, 2020 at 12:09:25PM +0900, Kyotaro Horiguchi wrote:
> > +    if (!XLogArchivingAlways() &&
> > +        GetRecoveryState() == RECOVERY_STATE_ARCHIVE)
> > 
> > Is rewritten as 
> > 
> > +    if (!XLogArchivingAlways() &&
> > +        GetDBState() > DB_IN_CRASH_RECOVERY)
> > 
> > FWIW, what annoyed me is there are three variables that are quite
> > similar but has different domains, ControlFile->state,
> > XLogCtl->SharedRecoveryState, and LocalRecoveryInProgress. I didn't
> > mind there were two, but three seems a bit too many to me.
> 
> That's actually the pattern I would avoid for clarity.  There is no
> need to add more dependencies to the entries of DBState for the sake
> of this patch, and this smells like a trap if more values are added to
> it in an order that does not match what we have been assuming in the
> context of this thread.

Yes. Anywaay that would be another issue, if it is an issue.

I'm fine with the current state.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center