On 2020/02/27 15:23, Kyotaro Horiguchi wrote:
At Thu, 27 Feb 2020 14:40:55 +0900, Fujii Masao <masao.fu...@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.

If so, what about adding the following at the top of
WaitForWALToBecomeAvailable()?

    if (!StandbyMode && currentSource == XLOG_FROM_STREAM)
         currentSource = 0;

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;

Sorry, I failed to understand why this change is necessary...
At least the comment that you added seems incorrect
because WAL streaming should not have started yet when
we reach the above point.

Regards,

--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters


Reply via email to