Date: Thu, 2 Apr 2020 17:37:44 +0200
From: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
 pgsql-bugs@lists.postgresql.org, michael@paquier.xyz
Subject: Re: [BUG] non archived WAL removed during production crash recovery
Message-ID: <20200402173744.0a504243@firost>
In-Reply-To: <d3e7d129-21ab-b98c-4b24-7a50db84ee3f@oss.nttdata.com>
References: <d0ef12c5-7e36-b9b3-c8ea-2e98d69d1317@oss.nttdata.com>
	<20200401181735.11100908@firost>
	<20200402.130443.1791362864704140391.horikyota.ntt@gmail.com>
	<20200402.130734.613319908534820379.horikyota.ntt@gmail.com>
	<20200402150234.278ae845@firost>
	<d3e7d129-21ab-b98c-4b24-7a50db84ee3f@oss.nttdata.com>
Organization: Dalibo
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 2 Apr 2020 23:58:00 +0900
Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

> On 2020/04/02 22:02, Jehan-Guillaume de Rorthais wrote:
> > On Thu, 02 Apr 2020 13:07:34 +0900 (JST)
> > Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
> >   
> >> Sorry, it was quite ambiguous.
> >>
> >> At Thu, 02 Apr 2020 13:04:43 +0900 (JST), Kyotaro Horiguchi
> >> <horikyota.ntt@gmail.com> wrote in  
> >>> At Wed, 1 Apr 2020 18:17:35 +0200, Jehan-Guillaume de Rorthais
> >>> <jgdr@dalibo.com> wrote in  
> >>>> Please, find in attachment a patch implementing this.  
> >>>
> >>> The patch partially reintroduces the issue the patch have
> >>> fixed. Specifically a standby running a crash recovery wrongly marks a
> >>> WAL file as ".ready" if it is extant in pg_wal without accompanied by
> >>> .ready file.  
> >>
> >> The patch partially reintroduces the issue the commit 78ea8b5daa have
> >> fixed. Specifically a standby running a crash recovery wrongly marks a
> >> WAL file as ".ready" if it is extant in pg_wal without accompanied by
> >> .ready file.  
> > 
> > As far as I understand StartupXLOG(), NOT_IN_RECOVERY and IN_CRASH_RECOVERY
> > are only set for production clusters, not standby ones.  
> 
> DB_IN_CRASH_RECOVERY can be set even in standby mode. For example,
> if you start the standby from the cold backup of the primary,

In cold backup? Then ControlFile->state == DB_SHUTDOWNED, right?

Unless I'm wrong, this should be catched by:

  if (ArchiveRecoveryRequested && ( [...] ||
	 ControlFile->state == DB_SHUTDOWNED))
  {
	InArchiveRecovery = true;
	if (StandbyModeRequested)
		StandbyMode = true;
  }

With InArchiveRecovery=true, we later set DB_IN_ARCHIVE_RECOVERY instead of
DB_IN_CRASH_RECOVERY.


> since InArchiveRecovery is false at the beginning of the recovery,
> DB_IN_CRASH_RECOVERY is set in that moment. But then after all the valid
> WAL in pg_wal have been replayed, InArchiveRecovery is set to true and
> DB_IN_ARCHIVE_RECOVERY is set.

However, I suppose this is true if you restore a backup from a snapshot
without backup_label, right?