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

At Tue, 21 Apr 2020 13:57:39 +0900, Michael Paquier <michael(at)paquier(dot)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