Bug#35387076 ndb_desc_extra occasionally fails in PB2 due to content mismatch

Also fixing :
  Bug#35416104 ndb_read_local occasionally fails in PB2 due to timeout
  Bug#34944798 ndb_binlog.ndb_autodiscover3 fails sporadically in PB2
  Bug#34196275 ndb_desc_extra fails with missing hashmap
  Bug#34196250 Test metadata_immediate_sync can fail in PB2
  Bug#33809408 Failure of the ndb_restore_schema_partitions test
  Bug#32368395 NDB_DESC UTILITY CAN FAIL ON DEVBLD
  Bug#35433318 PB2: ndb_rpl.ndb_rpl_sync sporadically fails to create hashmap

Problem

  - Transporters will only deliver a subset of signals until communication
    is enabled - other signals are dropped
  - Therefore it is important to only send general signals after
    communication is enabled as otherwise they may be dropped.
  - API nodes should not receive API_REGCONF from a data node until
    after communication is enabled, so can use this as a 'safe to proceed'
    indicator.
  - However if an API_REGREQ signal is received by a data node while it
    is enabling communication then it will currently send an immediate
    API_REGCONF - before communication is enabled.
  - If the API then sends general signals to the data node these race
    with the ENABLE_COMREQ and there is a chance that these signals are
    dropped
  - This can result in high level timeout errors such as error 4008 from
    early post-API_REGCONF activities (getTable, getHashmap etc)

Solution
  - Add assertion to ensure that data nodes only send API_REGCONF when
    API is in correct state
  - Add test insertion to give coverage of 'slow' ENABLE_COMORD to bias
    the race
  - Add testcase testNdbApi -n TestSlowConnectEnable to show :
    i)   4008 type scenario with error insertion
    ii)  Assertion failure with error insertion + assertion
    iii) No problem with fix
  - Fix by not sending API_REGCONF when waiting for ENABLE_COMCONF.

Change-Id: I796d9e5cab04a6048afa3b08c2558667a59331f9
Change-Id: I93d39e55b2c10ead06b419b69f254b45f6ad0bde
Change-Id: I277bd1e0e984358ef4cba1f5143d8be707d84a25

Author: frazerclement

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

@@ -30,6 +30,7 @@ Next DBDICT 6223
 Next DBDIH 7249
 Next DBTC 8123
 Next CMVMI 9000
+Next TRPMAN 9501
 Next BACKUP 10055
 Next PGMAN 11010
 Next DBTUX 12010

storage/ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2022, 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,
@@ -249,6 +249,12 @@ void Cmvmi::execNDB_TAMPER(Signal* signal)
     CLEAR_ERROR_INSERT_VALUE;
     sendSignal(TRPMAN_REF, GSN_NDB_TAMPER, signal, signal->getLength(),JBB);
   }
+  if (signal->theData[0] >= 9500 &&
+      signal->theData[0] <  9900)
+  {
+    /* Subrange for TRPMAN */
+    sendSignal(TRPMAN_REF, GSN_NDB_TAMPER, signal, signal->getLength(),JBB);
+  }
 }//execNDB_TAMPER()
 
 static Uint32 blocks[] =

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

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2022, 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,
@@ -3889,6 +3889,14 @@ void Qmgr::execAPI_REGREQ(Signal* signal)
      * should be able to discover what nodes that it is able to actually use.
      */
   }
+  if (apiNodePtr.p->phase == ZAPI_ACTIVATION_ONGOING)
+  {
+    jam();
+    /* Waiting for TRPMAN to finish enabling communication
+     * Must not send conf before then.
+     */
+    return;
+  }
 
   sendApiRegConf(signal, apiNodePtr.i);
 }//Qmgr::execAPI_REGREQ()
@@ -3986,6 +3994,17 @@ Qmgr::sendApiRegConf(Signal *signal, Uint32 node)
   const BlockReference ref = apiNodePtr.p->blockRef;
   ndbassert(ref != 0);
 
+  /* No Conf to be sent unless :
+   * - API node is ACTIVE
+   * - MGM node is ACTIVE | INACTIVE
+   * - Data node is shutting down
+   */
+  ndbassert(apiNodePtr.p->phase == ZAPI_ACTIVE ||
+            (apiNodePtr.p->phase == ZAPI_INACTIVE &&
+             getNodeInfo(apiNodePtr.i).getType() == NodeInfo::MGM) ||
+            (apiNodePtr.p->phase == ZAPI_INACTIVE &&
+             getNodeState().startLevel >= NodeState::SL_STOPPING_1));
+
   ApiRegConf * const apiRegConf = (ApiRegConf *)&signal->theData[0];
   apiRegConf->qmgrRef = reference();
   apiRegConf->apiHeartbeatFrequency = (chbApiDelay / 10);

storage/ndb/src/kernel/blocks/trpman.cpp

@@ -1,5 +1,5 @@
 /*
-  Copyright (c) 2011, 2021, Oracle and/or its affiliates.
+  Copyright (c) 2011, 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,
@@ -259,6 +259,20 @@ Trpman::execENABLE_COMREQ(Signal* signal)
   jamEntry();
   const EnableComReq *enableComReq = (const EnableComReq *)signal->getDataPtr();
 
+  if (ERROR_INSERTED(9500) &&
+      signal->getSendersBlockRef() != reference())
+  {
+    jam();
+    g_eventLogger->info("TRPMAN %u delaying ENABLE_COMREQ for 5s",
+                        instance());
+    sendSignalWithDelay(reference(),
+                        GSN_ENABLE_COMREQ,
+                        signal,
+                        5000,
+                        signal->getLength());
+    return;
+  }
+
   /* Need to copy out signal data to not clobber it with sendSignal(). */
   BlockReference senderRef = enableComReq->m_senderRef;
   Uint32 senderData = enableComReq->m_senderData;
@@ -499,6 +513,15 @@ Trpman::execNDB_TAMPER(Signal* signal)
 {
   jamEntry();
 #ifdef ERROR_INSERT
+  if (signal->getLength() == 1)
+  {
+    SET_ERROR_INSERT_VALUE(signal->theData[0]);
+  }
+  else
+  {
+    SET_ERROR_INSERT_VALUE2(signal->theData[0], signal->theData[1]);
+  }
+
   if (signal->theData[0] == 9003)
   {
     if (MAX_RECEIVED_SIGNALS < 1024)

storage/ndb/test/ndbapi/testNdbApi.cpp

@@ -7917,6 +7917,81 @@ int runCheckSlowCommit(NDBT_Context* ctx, NDBT_Step* step)
   return NDBT_OK;
 }
 
+int
+testSlowConnectEnable(NDBT_Context* ctx, NDBT_Step* step)
+{
+  /**
+   * Test behaviour of API client with slow connection
+   * enabling at the data node
+   */
+  NdbRestarter restarter;
+
+  ndbout_c("Delay ENABLE_COM on data nodes");
+  restarter.insertErrorInAllNodes(9500);
+
+  Ndb_cluster_connection* otherConnection = NULL;
+  Ndb* otherNdb = NULL;
+  int result = NDBT_FAILED;
+
+  do
+  {
+    ndbout_c("Setup new connection");
+    char connectString[256];
+    ctx->m_cluster_connection.get_connectstring(connectString,
+                                                sizeof(connectString));
+    otherConnection= new Ndb_cluster_connection(connectString);
+    if (otherConnection == NULL)
+    {
+      ndbout << "Could not create extra API connection" << endl;
+      break;
+    }
+
+    int rc = otherConnection->connect();
+    if (rc != 0)
+    {
+      ndbout << "Connection failed with " << rc << endl;
+      break;
+    }
+
+    if (otherConnection->wait_until_ready(30,30) != 0)
+    {
+      ndbout << "Connection wait until ready failed." << endl;
+      break;
+    }
+
+    ndbout_c("Connection ready");
+
+    otherNdb = new Ndb(otherConnection, "TEST_DB");
+    otherNdb->init();
+
+    if (otherNdb->waitUntilReady(30) != 0)
+    {
+      ndbout << "Ndb wait until ready failed." << endl;
+      break;
+    }
+
+    ndbout_c("Ndb ready");
+
+    const char* tabName = ctx->getTab()->getName();
+    if (otherNdb->getDictionary()->getTable(tabName) == NULL)
+    {
+      ndbout << "Get table failed with error "
+             << otherNdb->getNdbError() << endl;
+      break;
+    }
+
+    ndbout_c("Table retrieved");
+
+    result = NDBT_OK;
+  } while (0);
+
+  restarter.insertErrorInAllNodes(0);
+
+  delete otherNdb;
+  delete otherConnection;
+  return result;
+}
+
 
 NDBT_TESTSUITE(testNdbApi);
 TESTCASE("MaxNdb", 
@@ -8336,7 +8411,11 @@ TESTCASE("CheckSlowCommit",
   STEP(runCheckSlowCommit);
   FINALIZER(runDropTable);
 }
-
+TESTCASE("TestSlowConnectEnable",
+         "Test behaviour with slow connection enale")
+{
+  STEP(testSlowConnectEnable);
+}
 
 NDBT_TESTSUITE_END(testNdbApi);
 

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

@@ -248,3 +248,8 @@ max-time: 180
 cmd: testNodeRestart
 args: -n CheckGcpStopTimerDistributed T1
 max-time: 520
+
+cmd: testNdbApi
+args: -n TestSlowConnectEnable T1
+max-time: 600
+