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

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.
--
Michael