Re: Crash by targetted recovery

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: masao(dot)fujii(at)oss(dot)nttdata(dot)com
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Crash by targetted recovery
Date: 2020-02-27 06:23:07
Message-ID: 20200227.152307.953966656438810060.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Thu, 27 Feb 2020 14:40:55 +0900, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote in
>
>
> On 2020/02/27 12:48, Kyotaro Horiguchi wrote:
> > Hello.
> > We found that targetted promotion can cause an assertion failure. The
> > attached TAP test causes that.
> >
> >> TRAP: FailedAssertion("StandbyMode", File: "xlog.c", Line: 12078)
> > After recovery target is reached, StartupXLOG turns off standby mode
> > then refetches the last record. If the last record starts from the
> > previous WAL segment, the assertion failure is triggered.
>
> Good catch!
>
> > The wrong point is that StartupXLOG does random access fetching while
> > WaitForWALToBecomeAvailable is thinking it is still in streaming. I
> > think if it is called with random access mode,
> > WaitForWALToBecomeAvailable should move to XLOG_FROM_ARCHIVE even
> > though it is thinking that it is still reading from stream.
>
> I failed to understand why random access while reading from
> stream is bad idea. Could you elaborate why?

It seems to me the word "streaming" suggests that WAL record should be
read sequentially. Random access, which means reading from arbitrary
location, breaks a stream. (But the patch doesn't try to stop wal
sender if randAccess.)

> Isn't it sufficient to set currentSource to 0 when disabling
> StandbyMode?

I thought that and it should work, but I hesitated to manipulate on
currentSource in StartupXLOG. currentSource is basically a private
state of WaitForWALToBecomeAvailable. ReadRecord modifies it but I
think it's not good to modify it out of the the logic in
WaitForWALToBecomeAvailable. Come to think of that I got to think the
following part in ReadRecord should use randAccess instead..

xlog.c:4384
> /*
- * Before we retry, reset lastSourceFailed and currentSource
- * so that we will check the archive next.
+ * Streaming has broken, we retry from the same LSN.
> */
> lastSourceFailed = false;
- currentSource = 0;
+ private->randAccess = true;

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2020-02-27 06:31:58 Re: Allow auto_explain to log plans before queries are executed
Previous Message Pavel Stehule 2020-02-27 06:11:26 Re: Allow auto_explain to log plans before queries are executed