Bug #28698831 7.5.11/12: NODE RESTART GOES TO HANG STATE ..

Backport of bug fix pushed to 7.6 as part of pLCP WL.
(BUG#26166901  WL#10302 (8069): NODE RESTART GOES TO HANG STATE ...)

Created new testcase showing at least one aspect of the problem
which is fixed.

Testcase added to daily-devel--07

(cherry picked from commit 92b919993fc5d94236e98fb0f01ab4e63fc8717d)

Author: frazerclement

storage/ndb/src/kernel/blocks/ERROR_codes.txt

@@ -13,7 +13,7 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA
 
-Next QMGR 948
+Next QMGR 949
 Next NDBCNTR 1006
 Next NDBFS 2002
 Next DBACC 3002
@@ -259,6 +259,7 @@ QMGR
       3) ndb_mgm -e "2 error 946"
       4) ndb_mgm -e "all start"
 947:  Treat restart error due to a node disconnection as CRASH_INSERTION.
+948:  Cause node to crash when first hearing of a node failure
 
 ERROR CODES FOR TESTING NODE FAILURE, FAILURE IN COPY FRAGMENT PROCESS:
 -----------------------------------------------------------------------

storage/ndb/src/kernel/blocks/dbdih/Dbdih.hpp

@@ -1471,7 +1471,7 @@ class Dbdih: public SimulatedBlock {
 
   void packFragIntoPagesLab(Signal *, RWFragment* wf);
   void startNextChkpt(Signal *);
-  void failedNodeLcpHandling(Signal*, NodeRecordPtr failedNodePtr);
+  void failedNodeLcpHandling(Signal*, NodeRecordPtr failedNodePtr, bool &);
   void failedNodeSynchHandling(Signal *, NodeRecordPtr failedNodePtr);
   void checkCopyTab(Signal*, NodeRecordPtr failedNodePtr);
 

storage/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp

@@ -9183,6 +9183,7 @@ void Dbdih::execNODE_FAILREP(Signal* signal)
 
   const bool masterTakeOver = (oldMasterId != newMasterId);
 
+  bool check_more_start_lcp = false;
   for(i = 0; i < noOfFailedNodes; i++) {
     NodeRecordPtr failedNodePtr;
     failedNodePtr.i = failedNodes[i];
@@ -9216,7 +9217,7 @@ void Dbdih::execNODE_FAILREP(Signal* signal)
     // Functions that need to be called for all nodes.
     /*--------------------------------------------------*/
     checkStopMe(signal, failedNodePtr);
-    failedNodeLcpHandling(signal, failedNodePtr);
+    failedNodeLcpHandling(signal, failedNodePtr, check_more_start_lcp);
     startRemoveFailedNode(signal, failedNodePtr);
 
     /**
@@ -9244,6 +9245,23 @@ void Dbdih::execNODE_FAILREP(Signal* signal)
   }//if
 
   setGCPStopTimeouts();
+
+  /**
+   * Need to check if a node failed that was part of LCP. In this
+   * case we need to ensure that we don't get LCP hang by checking
+   * for sending of LCP_FRAG_ORD with last fragment flag set.
+   *
+   * This code cannot be called in master takeover case, in this
+   * case we restart the LCP in DIH entirely, so no need to worry
+   * here.
+   */
+  if (check_more_start_lcp &&
+      c_lcpMasterTakeOverState.state == LMTOS_IDLE)
+  {
+    jam();
+    ndbrequire(isMaster());
+    startNextChkpt(signal);
+  }
 }//Dbdih::execNODE_FAILREP()
 
 void Dbdih::checkCopyTab(Signal* signal, NodeRecordPtr failedNodePtr)
@@ -9509,7 +9527,9 @@ Dbdih::findTakeOver(Ptr<TakeOverRecord> & ptr, Uint32 failedNodeId)
   return false;
 }//Dbdih::findTakeOver()
 
-void Dbdih::failedNodeLcpHandling(Signal* signal, NodeRecordPtr failedNodePtr)
+void Dbdih::failedNodeLcpHandling(Signal* signal, 
+                                  NodeRecordPtr failedNodePtr,
+                                  bool& check_more_start_lcp)
 {
   jam();
   const Uint32 nodeId = failedNodePtr.i;
@@ -9554,6 +9574,18 @@ void Dbdih::failedNodeLcpHandling(Signal* signal, NodeRecordPtr failedNodePtr)
       ndbrequire(false);
       break;
     }//switch
+
+    jam();
+
+    /**
+     * It could be that the ongoing LCP is only waiting for our node, so
+     * it is important to here call checkStartMoreLcp. We need to go
+     * through all nodes first though to ensure that we don't call
+     * this and start checkpoints towards nodes already failed.
+     */
+    failedNodePtr.p->noOfQueuedChkpt = 0;
+    failedNodePtr.p->noOfStartedChkpt = 0;
+    check_more_start_lcp = true;
   }//if
 
   c_lcpState.m_participatingDIH.clear(failedNodePtr.i);
@@ -12820,6 +12852,11 @@ Dbdih::execDROP_TAB_REQ(Signal* signal)
 	  }
 	}
 	nodePtr.p->noOfQueuedChkpt = count;
+        if (nodePtr.p->noOfStartedChkpt == 0)
+        {
+          jam();
+          checkStartMoreLcp(signal, nodePtr.i);
+        }
       }
     }
   }
@@ -18296,6 +18333,7 @@ void Dbdih::startLcpRoundLoopLab(Signal* signal,
   for (nodePtr.i = 1; nodePtr.i < MAX_NDB_NODES; nodePtr.i++) {
     ptrAss(nodePtr, nodeRecord);
     if (nodePtr.p->nodeStatus == NodeRecord::ALIVE) {
+      jamLine(nodePtr.i);
       ndbrequire(nodePtr.p->noOfStartedChkpt == 0);
       ndbrequire(nodePtr.p->noOfQueuedChkpt == 0);
     }//if
@@ -18926,6 +18964,11 @@ void Dbdih::execLCP_FRAG_REP(Signal* signal)
       {
         jam();
         nodePtr.p->noOfQueuedChkpt--;
+        if (nodePtr.p->noOfStartedChkpt == 0)
+        {
+          jam();
+          checkStartMoreLcp(signal, nodePtr.i);
+        }
         return;
       }
     }

storage/ndb/src/kernel/blocks/qmgr/QmgrMain.cpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
+   Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -217,6 +217,8 @@ void Qmgr::execFAIL_REP(Signal* signal)
     failSource = refToNode(signal->getSendersBlockRef());
   }
 
+  CRASH_INSERTION(948);
+
   jamEntry();
   failReportLab(signal, failNodeId, failCause, failSource);
   return;

storage/ndb/test/ndbapi/testNodeRestart.cpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2016, Oracle and/or its affiliates. All rights reserved.
+   Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -7343,8 +7343,6 @@ runGcpStop(NDBT_Context* ctx, NDBT_Step* step)
 }
 
 
-static const Uint32 numTables = 20;
-
 int CMT_createTableHook(Ndb* ndb,
                         NdbDictionary::Table& table,
                         int when,
@@ -7370,6 +7368,7 @@ int CMT_createTableHook(Ndb* ndb,
 int createManyTables(NDBT_Context* ctx, NDBT_Step* step)
 {
   Ndb* pNdb = GETNDB(step);
+  const Uint32 numTables=ctx->getProperty("NumTables", Uint32(20));
 
   for (Uint32 tn = 0; tn < numTables; tn++)
   {
@@ -7396,6 +7395,8 @@ int dropManyTables(NDBT_Context* ctx, NDBT_Step* step)
 
   char buf[100];
 
+  const Uint32 numTables=ctx->getProperty("NumTables", Uint32(20));
+
   for (Uint32 tn = 0; tn < numTables; tn++)
   {
     BaseString::snprintf(buf, sizeof(buf),
@@ -7786,6 +7787,58 @@ int runRestartandCheckLCPRestored(NDBT_Context* ctx, NDBT_Step* step)
   return NDBT_OK;
 }
 
+
+int runNodeFailLcpStall(NDBT_Context* ctx, NDBT_Step* step)
+{
+  NdbRestarter restarter;
+  int master = restarter.getMasterNodeId();
+  int other = restarter.getRandomNodeSameNodeGroup(master, rand());
+
+  ndbout_c("Master %u  Other %u",
+           master, other);
+
+  ndbout_c("Stalling scans in node %u", other);
+  restarter.insertErrorInNode(other, 10039);
+
+  int dump[] = { 7099 };
+  ndbout_c("Triggering LCP");
+  restarter.dumpStateOneNode(master, dump, 1);
+
+  ndbout_c("Giving time for things to stall");
+  NdbSleep_MilliSleep(10000);
+
+  ndbout_c("Getting Master to kill other when Master LCP complete %u", master);
+  restarter.insertErrorInNode(master, 7178);
+
+  ndbout_c("Releasing scans in node %u", other);
+  restarter.insertErrorInNode(other, 0);
+
+  ndbout_c("Expect other node failure");
+  Uint32 retries=100;
+  while (restarter.getNodeStatus(other) == NDB_MGM_NODE_STATUS_STARTED)
+  {
+    if ((--retries) == 0)
+    {
+      ndbout_c("Timeout waiting for other node to restart");
+      return NDBT_FAILED;
+    }
+    NdbSleep_MilliSleep(500);
+  }
+
+  ndbout_c("Other node failed, now wait for it to restart");
+  restarter.insertErrorInNode(master, 0);
+
+  if (restarter.waitNodesStarted(&other, 1) != 0)
+  {
+    ndbout_c("Timed out waiting for restart");
+    return NDBT_FAILED;
+  }
+
+  ndbout_c("Restart succeeded");
+
+  return NDBT_OK;
+}
+
 NDBT_TESTSUITE(testNodeRestart);
 TESTCASE("NoLoad", 
 	 "Test that one node at a time can be stopped and then restarted "\
@@ -7951,8 +8004,9 @@ TESTCASE("RestartMasterNodeError",
 TESTCASE("GetTabInfoOverload",
          "Test behaviour of GET_TABINFOREQ overload + LCP + restart")
 {
+  TC_PROPERTY("NumTables", 20);
   INITIALIZER(createManyTables);
-  STEPS(runGetTabInfo, (int) numTables);
+  STEPS(runGetTabInfo, 20);
   STEP(runLCPandRestart);
   FINALIZER(dropManyTables);
 };
@@ -8478,6 +8532,14 @@ TESTCASE("RestoreOlderLCP",
   FINALIZER(runScanReadVerify);
   FINALIZER(runClearTable);
 }
+TESTCASE("NodeFailLcpStall",
+         "Check that node failure does not result in LCP stall")
+{
+  TC_PROPERTY("NumTables", Uint32(100));
+  INITIALIZER(createManyTables);
+  STEP(runNodeFailLcpStall);
+  FINALIZER(dropManyTables);
+}
 
 NDBT_TESTSUITE_END(testNodeRestart);
 

storage/ndb/test/run-test/daily-devel--07-tests.txt

@@ -93,3 +93,7 @@ cmd: test_event
 args: -n checkParallelTriggerDropReqHandling T1
 max-time: 120
 
+cmd: testNodeRestart
+args: -n NodeFailLcpStall T1
+max-time: 360
+