Bug#35389533 Backup won't start with NOWAIT option after restarting a data node

Problem:
A backup will not start if NOWAIT option is specified after a
data node is restarted.

Analysis:
1- if the restarted node is the master, master will eventually
change and start backup signals will end up in a wrong data
node(previous master)
2- if the backup is launched with nowait flag, the Management
Server will not wait for any reply from the backup block.

The way backup block has to report that self it not the master
is to send back a GSN_BACKUP_REF signal to Management Server.
So when both 1 and 2 are met, as Management Server is not
waiting for signals from backup the REF will be ignored and
consequently Management Server will not (re)try to perform the
start backup.

Solution:
From Management Server always wait for BACKUP_REF/BACKUP_CONF from
backup (even if nowait is specified). From backup side always send 1
signal to Management Server, GSN_BACKUP_REF if self is not master or
signal GSN_BACKUP_CONF otherwise.

Change-Id: Ie9c9163edf5b980676e72651f92cd73368d87fa9

Author: vinc13e

storage/ndb/include/kernel/signaldata/BackupSignalData.hpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2021, Oracle and/or its affiliates.
+   Copyright (c) 2003, 2023, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -50,6 +50,7 @@ class BackupReq {
   STATIC_CONST( SignalLength = 4 );
   STATIC_CONST( WAITCOMPLETED = 0x3 );
   STATIC_CONST( USE_UNDO_LOG = 0x4 );
+  STATIC_CONST( NOWAIT_REPLY = 0x20 );
 
 private:
   Uint32 senderData;

storage/ndb/include/ndb_version.h.in

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2004, 2021, Oracle and/or its affiliates.
+   Copyright (c) 2004, 2023, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -1144,4 +1144,56 @@ ndbd_support_waitgcp_shutdownsync(Uint32 x)
   return 1;
 }
 
+/*
+ * Data nodes reply to MGMD START BACKUP NOWAIT REQ from
+ * MGMD with BACKUP_REF/CONF.
+ */
+#define NDBD_START_BACKUP_NOWAIT_REPLY_75 NDB_MAKE_VERSION(7,5,31)
+#define NDBD_START_BACKUP_NOWAIT_REPLY_76 NDB_MAKE_VERSION(7,6,27)
+#define NDBD_START_BACKUP_NOWAIT_REPLY_80 NDB_MAKE_VERSION(8,0,35)
+#define NDBD_START_BACKUP_NOWAIT_REPLY_82 NDB_MAKE_VERSION(8,2,0)
+static
+inline
+int
+ndbd_start_backup_nowait_reply(Uint32 x)
+{
+  const Uint32 major = (x >> 16) & 0xFF;
+  const Uint32 minor = (x >>  8) & 0xFF;
+  if (major < 7)
+  {
+    return 0;
+  }
+  else if (major == 7)
+  {
+    if (minor < 5)
+    {
+      return 0;
+    }
+    else if (minor == 5)
+    {
+      return x >= NDBD_START_BACKUP_NOWAIT_REPLY_75;
+    }
+    else if (minor == 6)
+    {
+      return x >= NDBD_START_BACKUP_NOWAIT_REPLY_76;
+    }
+  }
+  else if (major == 8)
+  {
+    if (minor == 0)
+    {
+      return x >= NDBD_START_BACKUP_NOWAIT_REPLY_80;
+    }
+    else if (minor == 1)
+    {
+      return 0;
+    }
+    else
+    {
+      return x >= NDBD_START_BACKUP_NOWAIT_REPLY_82;
+    }
+  }
+  return 1;
+}
+
 #endif

storage/ndb/src/kernel/blocks/backup/Backup.cpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2021, Oracle and/or its affiliates.
+   Copyright (c) 2003, 2023, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -2501,7 +2501,19 @@ Backup::execBACKUP_REQ(Signal* signal)
                   BackupRef::IAmNotMaster);
     return;
   }//if
-
+  else if(flags & BackupReq::NOWAIT_REPLY)
+  {
+    /*
+     * Inform MgmtSrvr that I am the master node, this way MgmtSrvr can stop
+     * waiting for feedback from Backup when NOWAIT option is specified in the
+     * start backup command
+     */
+    BackupConf * conf = (BackupConf*)signal->getDataPtrSend();
+    conf->backupId = 0;
+    conf->senderData = req->senderData;
+    sendSignal(senderRef, GSN_BACKUP_CONF, signal,
+               BackupConf::SignalLength, JBB);
+  }
   if (c_defaults.m_diskless)
   {
     jam();
@@ -2642,17 +2654,25 @@ Backup::sendBackupRef(BlockReference senderRef, Uint32 flags, Signal *signal,
 		      Uint32 senderData, Uint32 errorCode)
 {
   jam();
-  if (SEND_BACKUP_STARTED_FLAG(flags))
+  if (SEND_BACKUP_STARTED_FLAG(flags) ||
+      (flags & BackupReq::NOWAIT_REPLY && errorCode == BackupRef::IAmNotMaster))
   {
     jam();
-    BackupRef* ref = (BackupRef*)signal->getDataPtrSend();
+    BackupRef *ref = (BackupRef *)signal->getDataPtrSend();
     ref->senderData = senderData;
     ref->errorCode = errorCode;
     ref->masterRef = numberToRef(BACKUP, getMasterNodeId());
     sendSignal(senderRef, GSN_BACKUP_REF, signal, BackupRef::SignalLength, JBB);
   }
-
-  if (errorCode != BackupRef::IAmNotMaster)
+  /*
+   * Log event if the error is other than IAmNotMaster,
+   * or, if error is IAmNotMaster, the NOWAIT option is set and data node
+   * version is >= 7.5.31 or >= 7.6.27 or >= 8.0.34  or >= 8.2.0 
+   * (see NDBD_START_BACKUP_NOWAIT_REPLY)
+   */
+  if ((errorCode != BackupRef::IAmNotMaster) ||
+      (errorCode == BackupRef::IAmNotMaster &&
+       !SEND_BACKUP_STARTED_FLAG(flags) && !(flags & BackupReq::NOWAIT_REPLY)))
   {
     jam();
     signal->theData[0] = NDB_LE_BackupFailedToStart;

storage/ndb/src/mgmsrv/MgmtSrvr.cpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2021, Oracle and/or its affiliates.
+   Copyright (c) 2003, 2023, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -1396,10 +1396,10 @@ MgmtSrvr::sendall_STOP_REQ(NodeBitmask &stoppedNodes,
 int
 MgmtSrvr::guess_master_node(SignalSender& ss)
 {
+  NodeId guess = m_master_node;
   /**
    * First check if m_master_node is started
    */
-  NodeId guess = m_master_node;
   if (guess != 0)
   {
     trp_node node = ss.getNodeInfo(guess);
@@ -1408,43 +1408,65 @@ MgmtSrvr::guess_master_node(SignalSender& ss)
   }
 
   /**
-   * Check for any started node
+   * Check started nodes based on dynamicId
    */
-  guess = 0;
-  while(getNextNodeId(&guess, NDB_MGM_NODE_TYPE_NDB))
+  Uint32 min = UINT32_MAX;
+  NodeId node_id = 0;
+  while(getNextNodeId(&node_id, NDB_MGM_NODE_TYPE_NDB))
   {
-    trp_node node = ss.getNodeInfo(guess);
-    if (node.m_state.startLevel == NodeState::SL_STARTED)
+    trp_node node = ss.getNodeInfo(node_id);
+    if(node.m_state.dynamicId < min)
     {
-      return guess;
+      if(node.m_state.startLevel == NodeState::SL_STARTED)
+      {
+        min = node.m_state.dynamicId;
+        guess = node_id;
+      }
     }
   }
+  //found
+  if(min < UINT32_MAX)
+    return guess;
 
   /**
-   * Check any confirmed node
+   * Check confirmed nodes based on dynamicId
    */
-  guess = 0;
-  while(getNextNodeId(&guess, NDB_MGM_NODE_TYPE_NDB))
+  node_id = 0;
+  while(getNextNodeId(&node_id, NDB_MGM_NODE_TYPE_NDB))
   {
-    trp_node node = ss.getNodeInfo(guess);
-    if (node.is_confirmed())
+    trp_node node = ss.getNodeInfo(node_id);
+    if(node.m_state.dynamicId < min)
     {
-      return guess;
+      if(node.is_confirmed())
+      {
+        min = node.m_state.dynamicId;
+        guess = node_id;
+      }
     }
   }
+  //found
+  if(min < UINT32_MAX)
+    return guess;
 
   /**
-   * Check any connected node
+   * Check connected nodes based on dynamicId
    */
-  guess = 0;
-  while(getNextNodeId(&guess, NDB_MGM_NODE_TYPE_NDB))
+  node_id = 0;
+  while(getNextNodeId(&node_id, NDB_MGM_NODE_TYPE_NDB))
   {
-    trp_node node = ss.getNodeInfo(guess);
-    if (node.is_connected())
+    trp_node node = ss.getNodeInfo(node_id);
+    if(node.m_state.dynamicId < min)
     {
-      return guess;
+      if(node.is_connected())
+      {
+        min = node.m_state.dynamicId;
+        guess = node_id;
+      }
     }
   }
+  //found
+  if(min < UINT32_MAX)
+    return guess;
 
   return 0; // give up
 }
@@ -3420,8 +3442,13 @@ MgmtSrvr::trp_deliver_signal(const NdbApiSignal* signal,
 
       /* Clear local nodeid reservation(if any) */
       release_local_nodeid_reservation(i);
-
       clear_connect_address_cache(i);
+
+      /* Clear m_master_node when master disconnects */
+      if(i == m_master_node)
+      {
+        m_master_node = 0;
+      }
     }
     return;
   }
@@ -4331,16 +4358,7 @@ MgmtSrvr::startBackup(Uint32& backupId, int waitCompleted, Uint32 input_backupId
   SignalSender ss(theFacade);
   ss.lock(); // lock will be released on exit
 
-  NodeId nodeId = m_master_node;
-  if (okToSendTo(nodeId, false) != 0)
-  {
-    bool next;
-    nodeId = m_master_node = 0;
-    while((next = getNextNodeId(&nodeId, NDB_MGM_NODE_TYPE_NDB)) == true &&
-          okToSendTo(nodeId, false) != 0);
-    if(!next)
-      return NO_CONTACT_WITH_DB_NODES;
-  }
+  NodeId nodeId = guess_master_node(ss);
 
   SimpleSignal ssig;
   BackupReq* req = CAST_PTR(BackupReq, ssig.getDataPtrSend());
@@ -4370,18 +4388,48 @@ MgmtSrvr::startBackup(Uint32& backupId, int waitCompleted, Uint32 input_backupId
   while (1) {
     if (do_send)
     {
+      nodeId = guess_master_node(ss);
+
+      if(waitCompleted == 0 &&
+          ndbd_start_backup_nowait_reply(getNodeInfo(nodeId).m_info.m_version))
+      {
+        req->flags |= BackupReq::NOWAIT_REPLY;
+      }
+      else
+      {
+        req->flags &= ~((Uint32)BackupReq::NOWAIT_REPLY);
+      }
+
       if (ss.sendSignal(nodeId, &ssig) != SEND_OK) {
 	return SEND_OR_RECEIVE_FAILED;
       }
-      if (waitCompleted == 0)
-	return 0;
+      if (waitCompleted == 0 &&
+          !ndbd_start_backup_nowait_reply(getNodeInfo(nodeId).m_info.m_version))
+      {
+        return 0;
+      }
       do_send = 0;
     }
     SimpleSignal *signal = ss.waitFor();
 
     int gsn = signal->readSignalNumber();
     switch (gsn) {
     case GSN_BACKUP_CONF:{
+
+  /*
+   * BACKUP NOWAIT case.
+   * BACKUP_CONF received from Backup. It is only used to confirm that
+   * the node is the master node and the backup can proceed.
+   * No more feedback expected from data node in this case.
+   */
+    if(waitCompleted == 0)
+    {
+      return 0;
+    }
+
+    /*
+     * BACKUP WAIT case.
+     */
       const BackupConf * const conf = 
 	CAST_CONSTPTR(BackupConf, signal->getDataPtr());
 #ifdef VM_TRACE