On 2020/03/06 10:29, Kyotaro Horiguchi wrote:
At Thu, 5 Mar 2020 19:51:11 +0900, Fujii Masao <masao.fu...@oss.nttdata.com>
wrote in
On 2020/03/05 12:08, Kyotaro Horiguchi wrote:
I understand that the reconnection for REDO record is useless. Ok I
take the !StandbyMode way.
The attached is the test script that is changed to count the added
test, and the slight revised main patch.
Thanks for the patch!
+ /* Wal receiver should not active when entring XLOG_FROM_ARCHIVE */
+ Assert(!WalRcvStreaming());
+1 to add this assertion check.
Isn't it better to always check this while trying to read WAL from
archive or pg_wal? So, what about the following change?
{
case XLOG_FROM_ARCHIVE:
case XLOG_FROM_PG_WAL:
+ /*
+ * WAL receiver should not be running while trying to
+ * read WAL from archive or pg_wal.
+ */
+ Assert(!WalRcvStreaming());
+
/* Close any old file we might have open. */
if (readFile >= 0)
(It seems retroverting to the first patch when I started this...)
The second place covers wider cases so I reverted the first place.
Thanks for updating the patch that way.
Not sure which patch you're mentioning, though.
Regarding 0003 patch, I added a bit more detail comments into
the patch so that we can understand the code more easily.
Updated version of 0003 patch attached. Barring any objection,
at first, I plan to commit this patch.
+ lastSourceFailed = false; /* We haven't failed on the new source */
Is this really necessary? Since ReadRecord() always reset
lastSourceFailed to false, it seems not necessary.
It's just to make sure. Actually lastSourceFailed is always false
when we get there. But when the source is switched, lastSourceFailed
should be changed to false as a matter of design. I'd like to do that
unless that harms.
OK.
- else if (currentSource == 0)
+ else if (currentSource == 0 ||
Though this is a *separate topic*, 0 should be XLOG_FROM_ANY?
There are some places where 0 is used as the value of currentSource.
IMO they should be updated so that XLOG_FROM_ANY is used instead of 0.
Yeah, I've thought that many times but have neglected since it is not
critical and trivial as a separate patch. I'd take the chance to do
that now. Another minor glitch is "int oldSource = currentSource;" it
is not debugger-friendly so I changed it to XLogSource. It is added
as a new patch file before the main patch.
There seems to be more other places where XLogSource and
XLOG_FROM_XXX are not used yet. For example, the initial values
of readSource and XLogReceiptSource, the type of argument
"source" in XLogFileReadAnyTLI() and XLogFileRead(), etc.
These also should be updated?
Regards,
--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 4361568882..b0e953f894 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7392,7 +7392,11 @@ StartupXLOG(void)
* We are now done reading the xlog from stream. Turn off streaming
* recovery to force fetching the files (which would be required at end
of
* recovery, e.g., timeline history file) from archive or pg_wal.
+ *
+ * Note that standby mode must be turned off after killing WAL receiver,
+ * i.e., calling ShutdownWalRcv().
*/
+ Assert(!WalRcvStreaming());
StandbyMode = false;
/*
@@ -11855,12 +11859,23 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool
randAccess,
* values for "check trigger", "rescan timelines", and "sleep" states,
* those actions are taken when reading from the previous source fails,
as
* part of advancing to the next state.
+ *
+ * If standby mode is turned off while reading WAL from stream, we move
+ * to XLOG_FROM_ARCHIVE and reset lastSourceFailed, to force fetching
+ * the files (which would be required at end of recovery, e.g., timeline
+ * history file) from archive or pg_wal. We don't need to kill WAL
receiver
+ * here because it's already stopped when standby mode is turned off at
+ * the end of recovery.
*-------
*/
if (!InArchiveRecovery)
currentSource = XLOG_FROM_PG_WAL;
- else if (currentSource == 0)
+ else if (currentSource == 0 ||
+ (!StandbyMode && currentSource == XLOG_FROM_STREAM))
+ {
+ lastSourceFailed = false;
currentSource = XLOG_FROM_ARCHIVE;
+ }
for (;;)
{
@@ -12054,6 +12069,12 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool
randAccess,
{
case XLOG_FROM_ARCHIVE:
case XLOG_FROM_PG_WAL:
+ /*
+ * WAL receiver must not be running when
reading WAL from
+ * archive or pg_wal.
+ */
+ Assert(!WalRcvStreaming());
+
/* Close any old file we might have open. */
if (readFile >= 0)
{
