Re: Crash by targetted recovery - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: Crash by targetted recovery
Date
Msg-id 20200227.152307.953966656438810060.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: Crash by targetted recovery  (Fujii Masao <masao.fujii@oss.nttdata.com>)
Responses Re: Crash by targetted recovery
Re: Crash by targetted recovery
List pgsql-hackers
At Thu, 27 Feb 2020 14:40:55 +0900, Fujii Masao <masao.fujii@oss.nttdata.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



pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: Re: Allow auto_explain to log plans before queries are executed
Next
From: Julien Rouhaud
Date:
Subject: Re: Allow auto_explain to log plans before queries are executed