Bug#32891206 CRASH IN NDB_DD_RESTART

blaudden · dahlerlend · commit 6c7292f35795 · 2021-06-21T15:48:11.000+02:00
Crash occurs when spurious schema operation reply is received from node
not registered as participant in the current schema operation. This will
in some cases (when client hasn't yet detected completion of schema
operation) cause the schema distribution coordinator to remove the
active schema operation a second time.

Fix by discarding replies from node not registered as participant. This
special case was actually already handled but didn't properly propagate
all the way to handling of active schema op.

Also remove two unused schema dist functions for starting schema
operations and document the two unsent schema operation codes used by
those functions as "never sent".

Change-Id: I57ca344ba22df582aa4a020145cc8fa343bf659c
diff --git a/storage/ndb/plugin/ha_ndbcluster_binlog.cc b/storage/ndb/plugin/ha_ndbcluster_binlog.cc
@@ -2569,7 +2569,7 @@ class Ndb_schema_event_handler {
       return;
     }
 
-    // Check if all coordinator completed and wake up client
+    // Check if coordinator completed and wake up client
     const bool coordinator_completed =
         ndb_schema_object->check_coordinator_completed();
 
@@ -3881,7 +3881,7 @@ class Ndb_schema_event_handler {
         rewrite_acl_change_for_server_log(query);
 
       ndb_log_verbose(19,
-                      "got schema event on '%s.%s(%u/%u)' query: '%s' "
+                      "Schema event on '%s.%s(%u/%u)' query: '%s' "
                       "type: %s(%d) node: %u slock: %x%08x",
                       schema->db, schema->name, schema->id, schema->version,
                       query.c_str(),
@@ -4215,8 +4215,14 @@ class Ndb_schema_event_handler {
       return;
     }
 
-    ndb_schema_object->result_received_from_node(participant_node_id, result,
-                                                 unpacked_message);
+    const bool participant_registered =
+        ndb_schema_object->result_received_from_node(participant_node_id,
+                                                     result, unpacked_message);
+    if (!participant_registered) {
+      ndb_log_info("Ignoring node: %d, not a registered participant",
+                   participant_node_id);
+      return;
+    }
 
     if (ndb_schema_object->check_all_participants_completed()) {
       // All participants have completed(or failed) -> send final ack
diff --git a/storage/ndb/plugin/ndb_schema_dist.cc b/storage/ndb/plugin/ndb_schema_dist.cc
@@ -624,20 +624,6 @@ bool Ndb_schema_dist_client::acl_notify(std::string user_list) {
                        SOT_ACL_SNAPSHOT);
 }
 
-bool Ndb_schema_dist_client::tablespace_changed(const char *tablespace_name,
-                                                int id, int version) {
-  DBUG_TRACE;
-  return log_schema_op(ndb_thd_query(m_thd), ndb_thd_query_length(m_thd), "",
-                       tablespace_name, id, version, SOT_TABLESPACE);
-}
-
-bool Ndb_schema_dist_client::logfilegroup_changed(const char *logfilegroup_name,
-                                                  int id, int version) {
-  DBUG_TRACE;
-  return log_schema_op(ndb_thd_query(m_thd), ndb_thd_query_length(m_thd), "",
-                       logfilegroup_name, id, version, SOT_LOGFILE_GROUP);
-}
-
 bool Ndb_schema_dist_client::create_tablespace(const char *tablespace_name,
                                                int id, int version) {
   DBUG_TRACE;
diff --git a/storage/ndb/plugin/ndb_schema_dist.h b/storage/ndb/plugin/ndb_schema_dist.h
@@ -45,8 +45,8 @@ enum SCHEMA_OP_TYPE {
   SOT_CREATE_DB = 5,
   SOT_ALTER_DB = 6,
   SOT_CLEAR_SLOCK = 7,
-  SOT_TABLESPACE = 8,
-  SOT_LOGFILE_GROUP = 9,
+  SOT_TABLESPACE = 8,     // Never sent since 8.0.14, still reserved
+  SOT_LOGFILE_GROUP = 9,  // Never sent since 8.0.14, still reserved
   SOT_RENAME_TABLE = 10,
   SOT_TRUNCATE_TABLE = 11,
   SOT_RENAME_TABLE_PREPARE = 12,
@@ -319,9 +319,6 @@ class Ndb_schema_dist_client {
                   bool participants_must_refresh);
   bool acl_notify(std::string user_list);
 
-  bool tablespace_changed(const char *tablespace_name, int id, int version);
-  bool logfilegroup_changed(const char *logfilegroup_name, int id, int version);
-
   bool create_tablespace(const char *tablespace_name, int id, int version);
   bool alter_tablespace(const char *tablespace_name, int id, int version);
   bool drop_tablespace(const char *tablespace_name, int id, int version);
diff --git a/storage/ndb/plugin/ndb_schema_object.cc b/storage/ndb/plugin/ndb_schema_object.cc
@@ -305,7 +305,7 @@ void NDB_SCHEMA_OBJECT::register_participants(
   ndbcluster::ndbrequire(nodes.size() == state.m_participants.size());
 }
 
-void NDB_SCHEMA_OBJECT::result_received_from_node(
+bool NDB_SCHEMA_OBJECT::result_received_from_node(
     uint32 participant_node_id, uint32 result,
     const std::string &message) const {
   std::lock_guard<std::mutex> lock_state(state.m_lock);
@@ -315,14 +315,15 @@ void NDB_SCHEMA_OBJECT::result_received_from_node(
     // Received reply from node not registered as participant, may happen
     // when a node hears the schema op but this node hasn't registered it as
     // subscriber yet.
-    return;
+    return false;  // Not registered
   }
 
   // Mark participant as completed and save result
   State::Participant &participant = it->second;
   participant.m_completed = true;
   participant.m_result = result;
   participant.m_message = message;
+  return true;
 }
 
 void NDB_SCHEMA_OBJECT::result_received_from_nodes(
diff --git a/storage/ndb/plugin/ndb_schema_object.h b/storage/ndb/plugin/ndb_schema_object.h
@@ -215,8 +215,10 @@ class NDB_SCHEMA_OBJECT {
      @param participant_node_id The nodeid of the node who reported result
      @param result The result received
      @param message The message describing the result if != 0
+
+     @return true if node was registered as participant, false otherwise
    */
-  void result_received_from_node(uint32 participant_node_id, uint32 result,
+  bool result_received_from_node(uint32 participant_node_id, uint32 result,
                                  const std::string &message) const;
 
   /**
