tmp10.2-MDEV-25768

10 files changed, 256 insertions, 36 deletions

diff --git a/mysql-test/suite/rpl/r/alter_gtid_slave_pos_engine.result b/mysql-test/suite/rpl/r/alter_gtid_slave_pos_engine.result
deleted file mode 100644
index de4c256c376..00000000000
--- a/mysql-test/suite/rpl/r/alter_gtid_slave_pos_engine.result
+++ /dev/null
@@ -1,26 +0,0 @@
-include/master-slave.inc
-[connection master]
-connection slave;
-# Initial gtid_slave_pos table engine is MyISAM
-#
-# Trying to alter gtid_slave_pos table while slave SQL thread is alive
-# should result in an error
-#
-# Slave_SQL_Running: Yes
-ALTER TABLE mysql.gtid_slave_pos ENGINE=innodb;
-ERROR HY000: This operation cannot be performed as you have a running slave ''; run STOP SLAVE '' first
-#
-# Altering gtid_slave_pos table while slave SQL thread is stopped should
-# succeed
-#
-include/stop_slave.inc
-# Slave_SQL_Running: No
-ALTER TABLE mysql.gtid_slave_pos ENGINE=innodb;
-include/start_slave.inc
-# New gtid_slave_pos table engine: InnoDB
-#
-# Cleanup
-#
-include/stop_slave.inc
-include/start_slave.inc
-include/rpl_end.inc
diff --git a/mysql-test/suite/rpl/t/alter_gtid_slave_pos_engine.test b/mysql-test/suite/rpl/t/alter_gtid_slave_pos_engine.test
index 0e44a932a4b..cec12df98c8 100644
--- a/mysql-test/suite/rpl/t/alter_gtid_slave_pos_engine.test
+++ b/mysql-test/suite/rpl/t/alter_gtid_slave_pos_engine.test
@@ -24,7 +24,7 @@
 --echo #
 --let slave_sql_running= query_get_value(SHOW SLAVE STATUS, Slave_SQL_Running, 1)
 --echo # Slave_SQL_Running: $slave_sql_running
---error 1198
+--error 1205
 ALTER TABLE mysql.gtid_slave_pos ENGINE=innodb;
 
 --echo #
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index 6a7ea117c84..03a102b5851 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -949,6 +949,10 @@ PSI_mutex_key key_LOCK_after_binlog_sync;
 PSI_mutex_key key_LOCK_prepare_ordered, key_LOCK_commit_ordered;
 PSI_mutex_key key_TABLE_SHARE_LOCK_share;
 
+#ifdef HAVE_REPLICATION
+PSI_mutex_key key_LOCK_alter_safety;
+#endif
+
 static PSI_mutex_info all_server_mutexes[]=
 {
 #ifdef HAVE_MMAP
@@ -983,6 +987,7 @@ static PSI_mutex_info all_server_mutexes[]=
   { &key_LOCK_manager, "LOCK_manager", PSI_FLAG_GLOBAL},
   { &key_LOCK_prepared_stmt_count, "LOCK_prepared_stmt_count", PSI_FLAG_GLOBAL},
   { &key_LOCK_rpl_status, "LOCK_rpl_status", PSI_FLAG_GLOBAL},
+  { &key_LOCK_alter_safety, "LOCK_alter_safety", PSI_FLAG_GLOBAL},
   { &key_LOCK_server_started, "LOCK_server_started", PSI_FLAG_GLOBAL},
   { &key_LOCK_status, "LOCK_status", PSI_FLAG_GLOBAL},
   { &key_LOCK_show_status, "LOCK_show_status", PSI_FLAG_GLOBAL},
@@ -4878,8 +4883,12 @@ static int init_thread_environment()
   pthread_attr_setscope(&connection_attrib, PTHREAD_SCOPE_SYSTEM);
 
 #ifdef HAVE_REPLICATION
+/* Or here! */
   rpl_init_gtid_slave_state();
   rpl_init_gtid_waiting();
+  register_alter_safety_check(
+      C_STRING_WITH_LEN("mysql"), rpl_gtid_slave_state_table_name.str,
+      rpl_gtid_slave_state_table_name.length, set_unsafe_gtid_slave_pos_error);
 #endif
 
   DBUG_RETURN(0);
@@ -5099,6 +5108,7 @@ static int init_server_components()
 
   my_uuid_init((ulong) (my_rnd(&sql_rand))*12345,12345);
 #ifdef HAVE_REPLICATION
+  /* Potentially here! */
   init_slave_list();
 #endif
   wt_init();
diff --git a/sql/mysqld.h b/sql/mysqld.h
index 44b0491f138..00e1894cbf9 100644
--- a/sql/mysqld.h
+++ b/sql/mysqld.h
@@ -318,6 +318,10 @@ extern PSI_rwlock_key key_rwlock_LOCK_grant, key_rwlock_LOCK_logger,
   key_rwlock_LOCK_sys_init_connect, key_rwlock_LOCK_sys_init_slave,
   key_rwlock_LOCK_system_variables_hash, key_rwlock_query_cache_query_lock;
 
+#ifdef HAVE_REPLICATION
+extern PSI_mutex_key key_LOCK_alter_safety;
+#endif
+
 #ifdef HAVE_MMAP
 extern PSI_cond_key key_PAGE_cond, key_COND_active, key_COND_pool;
 #endif /* HAVE_MMAP */
diff --git a/sql/rpl_gtid.h b/sql/rpl_gtid.h
index e4949ff0d39..a721a368678 100644
--- a/sql/rpl_gtid.h
+++ b/sql/rpl_gtid.h
@@ -314,4 +314,7 @@ extern int gtid_check_rpl_slave_state_table(TABLE *table);
 extern rpl_gtid *gtid_parse_string_to_list(const char *p, size_t len,
                                            uint32 *out_len);
 
+extern bool validate_alter_gtid_slave_pos(void);
+extern bool cleanup_alter_gtid_slave_pos(void);
+
 #endif  /* RPL_GTID_H */
diff --git a/sql/rpl_rli.cc b/sql/rpl_rli.cc
index 24c2d742753..59b907bc1b0 100644
--- a/sql/rpl_rli.cc
+++ b/sql/rpl_rli.cc
@@ -2299,6 +2299,12 @@ bool Relay_log_info::flush()
   DBUG_RETURN(error);
 }
 
+void set_unsafe_gtid_slave_pos_error()
+{
+  my_error(ER_SLAVE_MUST_STOP, MYF(0), (int) active_mi->connection_name.length,
+           active_mi->connection_name.str);
+}
+
 bool is_alter_allowed_by_rpl_state(const char *db_str, const char *table_str)
 {
   if (any_slave_sql_running() && active_mi &&
diff --git a/sql/rpl_rli.h b/sql/rpl_rli.h
index 58fd7b701a8..45e42a677ce 100644
--- a/sql/rpl_rli.h
+++ b/sql/rpl_rli.h
@@ -1006,4 +1006,6 @@ void delete_or_keep_event_post_apply(rpl_group_info *rgi,
 extern bool is_alter_allowed_by_rpl_state(const char *db_str,
                                           const char *table_str);
 
+extern void set_unsafe_gtid_slave_pos_error();
+
 #endif /* RPL_RLI_H */
diff --git a/sql/slave.cc b/sql/slave.cc
index 2ff1a0490e9..b89294666ab 100644
--- a/sql/slave.cc
+++ b/sql/slave.cc
@@ -4801,6 +4801,8 @@ pthread_handler_t handle_slave_sql(void *arg)
   const char *errmsg;
   rpl_group_info *serial_rgi;
   rpl_sql_thread_info sql_info(mi->rpl_filter);
+  MDL_request gtid_state_mdl_request;
+  MDL_request_list mdl_requests;
 
   // needs to call my_thread_init(), otherwise we get a coredump in DBUG_ stuff
   my_thread_init();
@@ -5060,6 +5062,19 @@ pthread_handler_t handle_slave_sql(void *arg)
   }
   mysql_mutex_unlock(&rli->data_lock);
 
+  gtid_state_mdl_request.init(MDL_key::TABLE, "mysql",
+                              rpl_gtid_slave_state_table_name.str,
+                              MDL_SHARED_WRITE, MDL_EXPLICIT);
+  mdl_requests.push_front(&gtid_state_mdl_request);
+
+  if (thd->mdl_context.acquire_locks(&mdl_requests,
+                                     thd->variables.lock_wait_timeout))
+  {
+    my_error(ER_CANT_LOCK, MYF(0));
+    goto err;
+  }
+  thd->mdl_context.set_needs_thr_lock_abort(TRUE);
+
   /* Read queries from the IO/THREAD until this thread is killed */
 
   thd->set_command(COM_SLAVE_SQL);
@@ -5153,6 +5168,11 @@ pthread_handler_t handle_slave_sql(void *arg)
     sql_print_information("master was %s:%d", mi->host, mi->port);
   }
 
+  if (gtid_state_mdl_request.ticket)
+  {
+    thd->mdl_context.release_all_locks_for_name(gtid_state_mdl_request.ticket);
+  }
+
  err_before_start:
 
   /*
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index ae4aa8c5cd5..d089d7020ab 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -8912,6 +8912,181 @@ simple_rename_or_index_change(THD *thd, TABLE_LIST *table_list,
   DBUG_RETURN(error != 0);
 }
 
+//static void alter_runtime_locks
+/*
+void free_domain_lookup_element(void *p)
+{
+  struct Binlog_gtid_state_validator::audit_elem *audit_elem=
+      (struct Binlog_gtid_state_validator::audit_elem *) p;
+  delete_dynamic(&audit_elem->late_gtids_previous);
+  delete_dynamic(&audit_elem->late_gtids_real);
+  my_free(audit_elem);
+}
+
+Binlog_gtid_state_validator::Binlog_gtid_state_validator()
+{
+  my_hash_init(PSI_INSTRUMENT_ME, &m_audit_elem_domain_lookup, &my_charset_bin, 32,
+               offsetof(struct audit_elem, domain_id), sizeof(uint32),
+               NULL, free_domain_lookup_element, HASH_UNIQUE);
+}
+
+Binlog_gtid_state_validator::~Binlog_gtid_state_validator()
+{
+  my_hash_free(&m_audit_elem_domain_lookup);
+}
+*/
+typedef struct _alter_safety_elem
+{
+  uint64 hash_val;
+  char *db;
+  size_t db_len;
+  char *table_name;
+  size_t table_name_len;
+  double mdl_timeout_override;
+  void (*error_func)();
+  mysql_mutex_t alter_tbl_lock;
+} alter_safety_elem;
+
+void free_alter_safety_elem(void *p)
+{
+  alter_safety_elem *e= (alter_safety_elem *) p;
+  my_free(e->db);
+  my_free(e->table_name);
+  my_free(e);
+}
+
+/*
+  Write up why this is needed..
+*/
+static uint64 hash_table_qualifier(const char *db, size_t db_len,
+                                   const char *table_name,
+                                   size_t table_name_len)
+{
+  uint64 hash_val= 47;
+  size_t i;
+
+  for (i= 0; i < db_len; i++)
+  {
+    hash_val ^= (*db++ << 1) | 1;
+  }
+
+  hash_val ^= ('.' << 1);
+
+  for (i= 0; i < table_name_len; i++)
+  {
+    hash_val ^= (*table_name++ << 1) | 1;
+  }
+
+  return hash_val;
+}
+
+static HASH alter_safety_hash;
+
+/*
+  Make an enum for tracking alter state?
+  ALTER_NOT_PROTECTED
+  ALTER_SAFE
+  ALTER UNSAFE
+*/
+
+int validate_alter_safety(const char *db, size_t db_len, const char *table_name, size_t table_name_len)
+{
+  uint64 hash_val= hash_table_qualifier(db, db_len, table_name, table_name_len);
+  alter_safety_elem *elem= (alter_safety_elem *) my_hash_search(
+      &alter_safety_hash, (const uchar *) &hash_val, 0);
+  /*
+    Double check that the hash corresponds to the intended database in case of
+    hash collision
+  */
+  if (elem && !(strncmp(elem->db, db, elem->db_len) ||
+                strncmp(elem->table_name, table_name, elem->table_name_len)))
+  {
+    if (mysql_mutex_trylock(&(elem->alter_tbl_lock)))
+    {
+      elem->error_func();
+      return 1;
+    }
+    mysql_mutex_unlock(&(elem->alter_tbl_lock));
+  }
+
+  return 0;
+}
+
+void set_table_unsafe_for_alter(const char *db, size_t db_len,
+                                const char *table_name, size_t table_name_len)
+{
+  uint64 hash_val= hash_table_qualifier(db, db_len, table_name, table_name_len);
+  alter_safety_elem *elem= (alter_safety_elem *) my_hash_search(
+      &alter_safety_hash, (const uchar *) &hash_val, 0);
+  DBUG_ASSERT(elem);
+  mysql_mutex_lock(&(elem->alter_tbl_lock));
+}
+
+double get_alter_mdl_timeout(THD *thd, const char *db, size_t db_len, const char *table_name,
+                       size_t table_name_len)
+{
+  uint64 hash_val= hash_table_qualifier(db, db_len, table_name, table_name_len);
+  alter_safety_elem *elem= (alter_safety_elem *) my_hash_search(
+      &alter_safety_hash, (const uchar *) &hash_val, 0);
+  if (!elem)
+    return thd->variables.lock_wait_timeout;
+  return elem->mdl_timeout_override;
+}
+
+void set_table_safe_for_alter(const char *db, size_t db_len,
+                                const char *table_name, size_t table_name_len)
+{
+  uint64 hash_val= hash_table_qualifier(db, db_len, table_name, table_name_len);
+  alter_safety_elem *elem= (alter_safety_elem *) my_hash_search(
+      &alter_safety_hash, (const uchar *) &hash_val, 0);
+  DBUG_ASSERT(elem);
+  mysql_mutex_unlock(&(elem->alter_tbl_lock));
+}
+
+void register_alter_safety_check(const char *db, size_t db_len, const char *table_name, size_t table_name_len,
+                                 void (*err_f)())
+{
+  static int32 is_hash_inited= 0;
+  static int32 false_ref= 0;
+
+  fprintf(stderr, "\n\nHash inited is %d\n",is_hash_inited);
+  if (my_atomic_cas32(&is_hash_inited, &false_ref, TRUE))
+  {
+    my_hash_init(&alter_safety_hash, &my_charset_bin, 4,
+                 offsetof(alter_safety_elem, hash_val),
+                 sizeof(decltype(alter_safety_elem::hash_val)), NULL,
+                        free_alter_safety_elem, HASH_UNIQUE);
+    fprintf(stderr, "We have initialized our alter_safety_hash\n");
+  }
+
+  uint64 table_hash=
+      hash_table_qualifier(db, db_len, table_name, table_name_len);
+
+  alter_safety_elem *new_el=
+      (alter_safety_elem *) my_malloc(sizeof(alter_safety_elem), MYF(MY_WME));
+  new_el->hash_val= table_hash;
+  new_el->db= (char *) my_malloc(db_len * sizeof(char) + 1,
+                                 MYF(MY_WME));
+  new_el->table_name=
+      (char *) my_malloc(table_name_len * sizeof(char) + 1, MYF(MY_WME));
+  new_el->db_len= db_len;
+  new_el->table_name_len= table_name_len;
+  strncpy(new_el->db, db, db_len);
+  new_el->db[db_len] = '\0';
+  strncpy(new_el->table_name, table_name, table_name_len);
+  new_el->table_name[table_name_len] = '\0';
+  new_el->mdl_timeout_override= 0;
+  new_el->error_func= err_f;
+  mysql_mutex_init(key_LOCK_alter_safety, &new_el->alter_tbl_lock,
+                   MY_MUTEX_INIT_FAST);
+
+  fprintf(stderr, "db %lu tl %lu\n", db_len, table_name_len);
+  fprintf(stderr, "Added new element for table %s.%s (%llu)\n", new_el->db,
+          new_el->table_name, new_el->hash_val);
+  my_hash_insert(&alter_safety_hash, (const uchar*) new_el);
+}
+
+
 
 /**
   Alter table
@@ -9059,6 +9234,9 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   Alter_table_ctx alter_ctx(thd, table_list, tables_opened, new_db, new_name);
 
   MDL_request target_mdl_request;
+  double mdl_wait_timeout= get_alter_mdl_timeout(
+      thd, alter_ctx.db, strlen(alter_ctx.db), alter_ctx.table_name,
+      strlen(alter_ctx.table_name));
 
   /* Check that we are not trying to rename to an existing table */
   if (alter_ctx.is_table_renamed())
@@ -9107,8 +9285,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
                                                  "", "",
                                                  MDL_INTENTION_EXCLUSIVE));
 
-      if (thd->mdl_context.acquire_locks(&mdl_requests,
-                                         thd->variables.lock_wait_timeout))
+      if (thd->mdl_context.acquire_locks(&mdl_requests, mdl_wait_timeout))
+                                         //thd->variables.lock_wait_timeout))
         DBUG_RETURN(true);
 
       DEBUG_SYNC(thd, "locked_table_name");
@@ -9205,14 +9383,28 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   }
 
 #ifdef HAVE_REPLICATION
-  if (thd->lex && thd->lex->sql_command == SQLCOM_ALTER_TABLE &&
-      table->s->db_type() != create_info->db_type &&
-      is_alter_allowed_by_rpl_state(alter_ctx.db, alter_ctx.table_name))
+  /*
+    rename to something like `unsafe_alter_locks` hashmap?
+  */
+  if (thd->lex && thd->lex->sql_command == SQLCOM_ALTER_TABLE)
   {
-    DBUG_PRINT("info", ("illegal alter"));
-    /* is_alter_allowed_by_rpl_state() sets the error details */
-    DBUG_RETURN(true);
+    if (validate_alter_safety(alter_ctx.db, strlen(alter_ctx.db),
+                          alter_ctx.table_name, strlen(alter_ctx.table_name)))
+    {
+      DBUG_PRINT("info", ("illegal alter"));
+      /* validate_alter_safety() sets the error details */
+      DBUG_RETURN(true);
+    }
   }
+
+//  if (thd->lex && thd->lex->sql_command == SQLCOM_ALTER_TABLE &&
+//      table->s->db_type() != create_info->db_type &&
+//      is_alter_allowed_by_rpl_state(alter_ctx.db, alter_ctx.table_name))
+//  {
+//    DBUG_PRINT("info", ("illegal alter"));
+//    /* is_alter_allowed_by_rpl_state() sets the error details */
+//    DBUG_RETURN(true);
+//  }
 #endif
 
   if (table->s->tmp_table == NO_TMP_TABLE)
@@ -9744,7 +9936,8 @@ do_continue:;
     */
     if (alter_info->requested_lock != Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE &&
         thd->mdl_context.upgrade_shared_lock(mdl_ticket, MDL_SHARED_NO_WRITE,
-                                             thd->variables.lock_wait_timeout))
+                                             mdl_wait_timeout))
+                                             //thd->variables.lock_wait_timeout))
       goto err_new_table_cleanup;
 
     DEBUG_SYNC(thd, "alter_table_copy_after_lock_upgrade");
diff --git a/sql/sql_table.h b/sql/sql_table.h
index aec783d4449..b904a1f6cd7 100644
--- a/sql/sql_table.h
+++ b/sql/sql_table.h
@@ -288,4 +288,12 @@ extern mysql_mutex_t LOCK_gdl;
 
 bool check_engine(THD *, const char *, const char *, HA_CREATE_INFO *);
 
+void register_alter_safety_check(const char *db, size_t db_len,
+                                 const char *table_name, size_t table_name_len,
+                                 void (*err_f)());
+void set_table_unsafe_for_alter(const char *db, size_t db_len,
+                                const char *table_name, size_t table_name_len);
+void set_table_safe_for_alter(const char *db, size_t db_len,
+                              const char *table_name, size_t table_name_len);
+
 #endif /* SQL_TABLE_INCLUDED */