add is_dd_engine_change_in_progress() to check dd_engine change (facebook#1378)

Summary:
Innodb SE need to know whether DD engine change is in progress for its handler API to return correct data. such as get_se_private_data() should return mysql.dd_properties table data is DD engine change in in progress and also DDSE is rocksdb.

Expose is_dd_engine_change information in dd.h. also update other places to use is_dd_engine_change variable to check whether DD engine change is in progress.

Pull Request resolved: facebook#1378

Differential Revision: D50715910

Author: luqun

sql/dd/dd.h

@@ -78,6 +78,12 @@ class Dictionary *get_dictionary();
 template <typename X>
 X *create_object();
 
+/**
+  Whether DD engine is changing in progress,such as INNODB DDSE to MyRocks DDSE
+  or vice versa.
+*/
+bool is_dd_engine_change_in_progress();
+
 ///////////////////////////////////////////////////////////////////////////
 
 }  // namespace dd

sql/dd/impl/bootstrap/bootstrapper.cc

@@ -1895,7 +1895,8 @@ bool sync_meta_data(THD *thd) {
 
 bool update_properties(THD *thd, const std::set<String_type> *create_set,
                        const std::set<String_type> *remove_set,
-                       const String_type &target_table_schema_name) {
+                       const String_type &target_table_schema_name,
+                       std::set<String_type> *innodb_set) {
   /*
     Populate the dd properties with the SQL DDL and SE private data.
     Store meta data of non-inert tables only.
@@ -1953,6 +1954,10 @@ bool update_properties(THD *thd, const std::set<String_type> *create_set,
       // All non-abandoned tables should have a table object present.
       assert(dd_table != nullptr);
 
+      // Record all innodb table name
+      if (innodb_set != nullptr && dd_table->engine() == "InnoDB") {
+        innodb_set->insert(dd_table->name());
+      }
       std::unique_ptr<dd::Properties> tbl_props(
           dd::Properties::parse_properties(""));
 

sql/dd/impl/bootstrap/bootstrapper.h

@@ -383,12 +383,15 @@ bool sync_meta_data(THD *thd);
                                     version of DD.
   @param target_table_schema_name   Schema name in which the final changes are
                                     required.
+  @param innodb_set                 A set of create_set table names whose engine
+                                    is innodb
 
   @return       Upon failure, return true, otherwise false.
 */
 bool update_properties(THD *thd, const std::set<String_type> *create_set,
                        const std::set<String_type> *remove_set,
-                       const String_type &target_table_schema_name);
+                       const String_type &target_table_schema_name,
+                       std::set<String_type> *innodb_set = nullptr);
 
 /**
   Updates the DD Version in the DD_properties table to the current version.

sql/dd/impl/dd.cc

@@ -23,6 +23,7 @@
 #include "sql/dd/dd.h"
 
 #include "sql/dd/cache/object_registry.h"
+#include "sql/dd/impl/bootstrap/bootstrap_ctx.h"        // dd::DD_bootstrap_ctx
 #include "sql/dd/impl/cache/shared_dictionary_cache.h"  // dd::cache::Shared_...
 #include "sql/dd/impl/dictionary_impl.h"                // dd::Dictionary_impl
 #include "sql/dd/impl/system_registry.h"                // dd::System_tables
@@ -79,6 +80,10 @@ X *create_object() {
   return dynamic_cast<X *>(new (std::nothrow) typename X::Impl());
 }
 
+bool is_dd_engine_change_in_progress() {
+  return bootstrap::DD_bootstrap_ctx::instance().is_dd_engine_change();
+}
+
 template Charset_impl *create_object<Charset_impl>();
 template Collation *create_object<Collation>();
 template Collation_impl *create_object<Collation_impl>();

sql/dd/impl/upgrade/dd.cc

@@ -1083,7 +1083,8 @@ bool update_object_ids(THD *thd, const std::set<String_type> &create_set,
 */
 bool update_myrocks_table_names(THD *thd, const String_type &old_schema_name,
                                 const String_type &new_schema_name,
-                                const std::set<String_type> &table_names) {
+    const std::set<String_type> &table_names,
+    const std::set<String_type> &filter_table_names) {
   assert(default_dd_storage_engine == DEFAULT_DD_ROCKSDB);
   assert(bootstrap::DD_bootstrap_ctx::instance().is_dd_engine_change());
 
@@ -1097,8 +1098,12 @@ bool update_myrocks_table_names(THD *thd, const String_type &old_schema_name,
     assert(false);
     return true;
   }
+
   bool err = false;
   for (const auto &table_name : table_names) {
+    // Skip some tables, such as InnoDB tables
+    if (filter_table_names.find(table_name) != filter_table_names.cend())
+      continue;
     char old_table[FN_REFLEN + 1];
     char new_table[FN_REFLEN + 1];
     // old table full name under old schema name
@@ -1181,7 +1186,7 @@ bool upgrade_dd_properties_table(THD *thd, Object_id mysql_schema_id,
       !dd::end_transaction(thd, false) &&
       update_myrocks_table_names(thd, target_table_schema_name,
                                  MYSQL_SCHEMA_NAME.str,
-                                 {dd_property_table_name})) {
+                                 {dd_property_table_name}, {})) {
     LogErr(ERROR_LEVEL, ER_LOG_PRINTF_MSG,
            "Failed to update myrocks table name for dd_properties table");
     return true;
@@ -1284,8 +1289,9 @@ bool upgrade_tables(THD *thd) {
     - Finally, update the version numbers and commit. In update_versions(),
       the atomic switch will either be committed.
   */
-  if (update_properties(thd, &create_set, &remove_set,
-                        target_table_schema_name) ||
+  std::set<String_type> innodb_set = {};
+  if (update_properties(thd, &create_set, &remove_set, target_table_schema_name,
+                        &innodb_set) ||
       update_object_ids(thd, create_set, remove_set, mysql_schema_id,
                         target_table_schema_id, target_table_schema_name,
                         actual_table_schema_id))
@@ -1296,7 +1302,8 @@ bool upgrade_tables(THD *thd) {
       (default_dd_storage_engine == DEFAULT_DD_ROCKSDB) &&
       !dd::end_transaction(thd, false) &&
       update_myrocks_table_names(thd, target_table_schema_name,
-                                 MYSQL_SCHEMA_NAME.str, create_set)) {
+                                 MYSQL_SCHEMA_NAME.str, create_set,
+                                 innodb_set)) {
     LogErr(ERROR_LEVEL, ER_LOG_PRINTF_MSG,
            "Failed to update myrocks table name for non-dd_properties tables");
     return true;

sql/sql_table.cc

@@ -18998,6 +18998,7 @@ static bool check_engine(THD *thd, const char *db_name, const char *table_name,
   handlerton **new_engine = &create_info->db_type;
 
   if (!(create_info->options & HA_LEX_CREATE_TMP_TABLE) && !opt_initialize &&
+      !dd::is_dd_engine_change_in_progress() &&
       ha_check_user_table_blocked(thd, *new_engine, db_name)) {
     handlerton *default_engine = ha_default_handlerton(thd);
     bool no_substitution = (!is_engine_substitution_allowed(thd));

storage/innobase/handler/ha_innodb.cc

@@ -15164,7 +15164,9 @@ bool ha_innobase::get_se_private_data(dd::Table *dd_table, bool reset) {
     dict_sys_t::s_dd_table_ids.clear();
   }
 
-  if (!innobase_is_ddse()) {
+  // Similar to boostrap, During DDSE change, SQL layer may also ask
+  // se_private_data for other DD tables
+  if (!innobase_is_ddse() && !dd::is_dd_engine_change_in_progress()) {
     if (dd_table->name() == "dd_properties_placeholder") {
       n_tables = 0;
       n_indexes = 0;