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

At Thu, 2 Apr 2020 14:19:15 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in 
> On 2020/04/02 13:07, Kyotaro Horiguchi 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.
> 
> On second thought, I think that we should discuss what the desirable
> behavior is before the implentation. Especially what's unclear to me

Agreed.

> is whether to remove such WAL files in archive recovery case with
> archive_mode=on. Those WAL files would be required when recovering
> from the backup taken before that archive recovery happens.
> So it seems unsafe to remove them in that case.

I'm not sure I'm getting the intention correctly, but I think it
responsibility of the operator to provide a complete set of archived
WAL files for a backup.  Could you elaborate what operation steps are
you assuming of?

> Therefore, IMO that the patch should change the code so that
> no unarchived WAL files are removed not only in crash recovery
> but also archive recovery. Thought?

Agreed if "an unarchived WAL" means "a WAL file that is marked .ready"
and it should be archived immediately.  My previous mail is written
based on the same thought.

In a very narrow window, if server crashed or killed after a segment
is finished but before marking the file as .ready, the file doesn't
have .ready but should be archived. If we need to get rid of such a
window, it would help to mark a WAL file as ".busy" at creation time.

> Of course, this change would lead to the issue that the past
> unarchived
> WAL files keep remaining in the case of warm-standby using archive
> recovery. But this issue looks unavoidable. If users want to avoid
> that,
> archive_mode should be set to always.
> 
> Also I'm a bit wondering if it's really safe to remove such unarchived
> WAL files even in the standby case with archive_mode=on. I would need
> more time to think that.
> 
> >> Perhaps checking '.ready' before the checking for archive-mode would
> >> be sufficient.
> >>
> >>> Plus, I added a second commit to add one test in regard with this bug.
> >>>
> >>>> Another is to make the startup process remove .ready file if
> >>>> necessary.
> >>>
> >>> I'm not sure to understand this one.
> 
> I was thinking to make the startup process remove such unarchived WAL
> files
> if archive_mode=on and StandbyModeRequested/ArchiveRecoveryRequested
> is true.

As mentioned above, I don't understand the point of preserving WAL
files that are either marked as .ready or not marked at all on a
standby with archive_mode=on.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center