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

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: jgdr@dalibo.com, masao.fujii@oss.nttdata.com, pgsql-bugs@lists.postgresql.org
Date: 2020-04-21T04:57:39Z
Lists: pgsql-bugs, pgsql-hackers
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

Commits

  1. Fix interpolation in test name.

  2. Add more TAP coverage for archive status with crash recovery of standbys

  3. Fix handling of WAL segments ready to be archived during crash recovery

  4. Fix WAL recycling on standbys depending on archive_mode