MDEV-25089 : Assertion `error.len > 0' failed in galera::ReplicatorSM…

…M::handle_apply_error()

Problem is that Galera starts TOI (total order isolation) i.e.
it sends query to all nodes. Later it is discovered that
used engine or other feature is not supported by Galera.
Because TOI is executed parallelly in all nodes appliers
could execute given TOI and ignore the error and
start inconsistency voting causing node to leave from
cluster or we might have a crash as reported.

For example SEQUENCE engine does not support GEOMETRY data
type causing either inconsistency between nodes (because
some errors are ignored on applier) or crash.

Fixed my adding new function wsrep_check_support to check
can Galera support provided CREATE TABLE/SEQUENCE before TOI is
started and if not clear error message is provided to
the user.

Currently, not supported cases:

* CREATE TABLE ... AS SELECT when streaming replication is used
* CREATE TABLE ... WITH SYSTEM VERSIONING AS SELECT
* CREATE TABLE ... ENGINE=SEQUENCE
* CREATE SEQUENCE ... ENGINE!=InnoDB
* ALTER TABLE t ... ENGINE!=InnoDB where table t is SEQUENCE

Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>

mysql-test/suite/galera/r/MDEV-24143.result

@@ -14,7 +14,7 @@ c1
 INSERT INTO t1 VALUES (4),(3),(1),(2);
 ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
 CREATE TABLE t1 (pk INT PRIMARY KEY, b INT) ENGINE=SEQUENCE;
-ERROR 42S01: Table 't1' already exists
+ERROR 42000: This version of MariaDB doesn't yet support 'non-InnoDB sequences in Galera cluster'
 ALTER TABLE t1 DROP COLUMN c2;
 ERROR 42000: Can't DROP COLUMN `c2`; check that it exists
 SELECT get_lock ('test', 1.5);

mysql-test/suite/galera/r/galera_sequence_engine.result

@@ -0,0 +1,12 @@
+connection node_2;
+connection node_1;
+SET GLOBAL wsrep_ignore_apply_errors=0;
+SET SESSION AUTOCOMMIT=0;
+SET SESSION max_error_count=0;
+CREATE TABLE t0 (id GEOMETRY,parent_id GEOMETRY)ENGINE=SEQUENCE;
+ERROR 42000: This version of MariaDB doesn't yet support 'non-InnoDB sequences in Galera cluster'
+connection node_2;
+SHOW CREATE TABLE t0;
+ERROR 42S02: Table 'test.t0' doesn't exist
+connection node_1;
+SET GLOBAL wsrep_ignore_apply_errors=DEFAULT;

mysql-test/suite/galera/r/mdev-31285.result

@@ -1,23 +1,8 @@
 connection node_2;
 connection node_1;
 connection node_1;
-connection node_2;
-connection node_1;
 CREATE TABLE t ENGINE=InnoDB WITH SYSTEM VERSIONING AS SELECT 1 AS i;
-SHOW CREATE TABLE t;
-Table	Create Table
-t	CREATE TABLE `t` (
-  `i` int(1) NOT NULL
-) ENGINE=InnoDB DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci WITH SYSTEM VERSIONING
-SELECT * from t;
-i
-1
-DROP TABLE IF EXISTS t;
-COMMIT;
+ERROR 42000: This version of MariaDB doesn't yet support 'SYSTEM VERSIONING AS SELECT in Galera cluster'
 connection node_2;
-SET SESSION wsrep_sync_wait=0;
-Killing server ...
-Starting server ...
-connection node_2;
-call mtr.add_suppression("WSREP: Event .*Write_rows_v1 apply failed:.*");
-call mtr.add_suppression("SREP: Failed to apply write set: gtid:.*");
+SHOW CREATE TABLE t;
+ERROR 42S02: Table 'test.t' doesn't exist

mysql-test/suite/galera/t/MDEV-24143.test

@@ -11,7 +11,11 @@ SET SESSION autocommit=0;
 SELECT * FROM t1 WHERE c1 <=0 ORDER BY c1 DESC;
 --error ER_LOCK_DEADLOCK
 INSERT INTO t1 VALUES (4),(3),(1),(2);
---error ER_TABLE_EXISTS_ERROR
+#
+# This is because support for CREATE TABLE ENGINE=SEQUENCE
+# is done before we check does table exists already.
+#
+--error ER_NOT_SUPPORTED_YET
 CREATE TABLE t1 (pk INT PRIMARY KEY, b INT) ENGINE=SEQUENCE;
 --error ER_CANT_DROP_FIELD_OR_KEY
 ALTER TABLE t1 DROP COLUMN c2;

mysql-test/suite/galera/t/galera_sequence_engine.test

@@ -0,0 +1,16 @@
+--source include/galera_cluster.inc
+--source include/have_sequence.inc
+
+SET GLOBAL wsrep_ignore_apply_errors=0;
+SET SESSION AUTOCOMMIT=0;
+SET SESSION max_error_count=0;
+--error ER_NOT_SUPPORTED_YET
+CREATE TABLE t0 (id GEOMETRY,parent_id GEOMETRY)ENGINE=SEQUENCE;
+
+--connection node_2
+--error ER_NO_SUCH_TABLE
+SHOW CREATE TABLE t0;
+
+--connection node_1
+SET GLOBAL wsrep_ignore_apply_errors=DEFAULT;
+

mysql-test/suite/galera/t/mdev-31285.test

@@ -1,34 +1,15 @@
 --source include/galera_cluster.inc
 
---let $node_1 = node_1
---let $node_2 = node_2
---source include/auto_increment_offset_save.inc
-
 --connection node_1
-CREATE TABLE t ENGINE=InnoDB WITH SYSTEM VERSIONING AS SELECT 1 AS i;
-SHOW CREATE TABLE t;
-SELECT * from t;
-DROP TABLE IF EXISTS t;
-COMMIT;
-
 #
-# Restart node_2, force SST because database is inconsistent compared to node_1
+# Below should not cause nodes to be inconsistent (they could if we
+# allow TOI as some error are ignored on applier
 #
---connection node_2
-SET SESSION wsrep_sync_wait=0;
---source include/kill_galera.inc
---remove_file $MYSQLTEST_VARDIR/mysqld.2/data/grastate.dat
---echo Starting server ...
-let $restart_noprint=2;
---source include/start_mysqld.inc
---let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size';
---source include/wait_condition.inc
-
---let $wait_condition = SELECT VARIABLE_VALUE = 'ON' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_ready';
---source include/wait_condition.inc
+--error ER_NOT_SUPPORTED_YET
+CREATE TABLE t ENGINE=InnoDB WITH SYSTEM VERSIONING AS SELECT 1 AS i;
 
 --connection node_2
-call mtr.add_suppression("WSREP: Event .*Write_rows_v1 apply failed:.*");
-call mtr.add_suppression("SREP: Failed to apply write set: gtid:.*");
+--error ER_NO_SUCH_TABLE
+SHOW CREATE TABLE t;
+
 
---source include/auto_increment_offset_restore.inc

sql/sql_sequence.cc

@@ -27,6 +27,9 @@
 #include "sql_acl.h"
 #ifdef WITH_WSREP
 #include "wsrep_mysqld.h"
+bool wsrep_check_sequence(THD* thd,
+                          const sequence_definition *seq,
+                          const bool used_engine);
 #endif
 
 struct Field_definition
@@ -945,7 +948,8 @@ bool Sql_cmd_alter_sequence::execute(THD *thd)
 #ifdef WITH_WSREP
   if (WSREP(thd) && wsrep_thd_is_local(thd))
   {
-    if (wsrep_check_sequence(thd, new_seq))
+    const bool used_engine= lex->create_info.used_fields & HA_CREATE_USED_ENGINE;
+    if (wsrep_check_sequence(thd, new_seq, used_engine))
       DBUG_RETURN(TRUE);
 
     if (wsrep_to_isolation_begin(thd, first_table->db.str,

sql/sql_table.cc

@@ -5343,18 +5343,21 @@ int mysql_create_table_no_lock(THD *thd, const LEX_CSTRING *db,
 #ifdef WITH_WSREP
 /** Additional sequence checks for Galera cluster.
 
-@param thd    thread handle
-@param seq    sequence definition
+@param thd         thread handle
+@param seq         sequence definition
+@param used_engine create used ENGINE=
 @retval false success
 @retval true  failure
 */
-bool wsrep_check_sequence(THD* thd, const sequence_definition *seq)
+bool wsrep_check_sequence(THD* thd,
+                          const sequence_definition *seq,
+                          const bool used_engine)
 {
     enum legacy_db_type db_type;
 
     DBUG_ASSERT(WSREP(thd));
 
-    if (thd->lex->create_info.used_fields & HA_CREATE_USED_ENGINE)
+    if (used_engine)
     {
       db_type= thd->lex->create_info.db_type->db_type;
     }
@@ -5385,6 +5388,57 @@ bool wsrep_check_sequence(THD* thd, const sequence_definition *seq)
 
     return (false);
 }
+
+/** Additional CREATE TABLE/SEQUENCE checks for Galera cluster.
+
+@param thd         thread handle
+@param wsrep_ctas  CREATE TABLE AS SELECT ?
+@param used_engine CREATE TABLE ... ENGINE = ?
+@param create_info Create information
+
+@retval false      Galera cluster does support used clause
+@retval true       Galera cluster does not support used clause
+*/
+static
+bool wsrep_check_support(THD* thd,
+                         const bool wsrep_ctas,
+                         const bool used_engine,
+                         const HA_CREATE_INFO* create_info)
+{
+  /* CREATE TABLE ... AS SELECT */
+  if (wsrep_ctas &&
+      thd->variables.wsrep_trx_fragment_size > 0)
+  {
+    my_message(ER_NOT_ALLOWED_COMMAND,
+               "CREATE TABLE AS SELECT is not supported with streaming replication",
+               MYF(0));
+    return true;
+  }
+  /* CREATE TABLE .. WITH SYSTEM VERSIONING AS SELECT
+     is not supported in Galera cluster.
+  */
+  if (wsrep_ctas &&
+      create_info->versioned())
+  {
+    my_error(ER_NOT_SUPPORTED_YET, MYF(0),
+             "SYSTEM VERSIONING AS SELECT in Galera cluster");
+    return true;
+  }
+  /*
+    CREATE TABLE ... ENGINE=SEQUENCE is not supported in
+    Galera cluster.
+    CREATE SEQUENCE ... ENGINE=xxx Galera cluster supports
+    only InnoDB-sequences.
+  */
+  if (((used_engine && create_info->db_type &&
+       (create_info->db_type->db_type == DB_TYPE_SEQUENCE ||
+        create_info->db_type->db_type >= DB_TYPE_FIRST_DYNAMIC)) ||
+       thd->lex->sql_command == SQLCOM_CREATE_SEQUENCE) &&
+      wsrep_check_sequence(thd, create_info->seq_create_info, used_engine))
+    return true;
+
+  return false;
+}
 #endif /* WITH_WSREP */
 
 /**
@@ -5442,15 +5496,6 @@ bool mysql_create_table(THD *thd, TABLE_LIST *create_table,
   if (!opt_explicit_defaults_for_timestamp)
     promote_first_timestamp_column(&alter_info->create_list);
 
-#ifdef WITH_WSREP
-  if (thd->lex->sql_command == SQLCOM_CREATE_SEQUENCE &&
-      WSREP(thd) && wsrep_thd_is_local_toi(thd))
-  {
-    if (wsrep_check_sequence(thd, create_info->seq_create_info))
-      DBUG_RETURN(true);
-  }
-#endif /* WITH_WSREP */
-
   /* We can abort create table for any table type */
   thd->abort_on_warning= thd->is_strict_mode();
 
@@ -9764,6 +9809,7 @@ bool mysql_alter_table(THD *thd, const LEX_CSTRING *new_db,
     TODO: this design is obsolete and will be removed.
   */
   int table_kind= check_if_log_table(table_list, FALSE, NullS);
+  const bool used_engine= create_info->used_fields & HA_CREATE_USED_ENGINE;
 
   if (table_kind)
   {
@@ -9775,7 +9821,7 @@ bool mysql_alter_table(THD *thd, const LEX_CSTRING *new_db,
     }
 
     /* Disable alter of log tables to unsupported engine */
-    if ((create_info->used_fields & HA_CREATE_USED_ENGINE) &&
+    if ((used_engine) &&
         (!create_info->db_type || /* unknown engine */
          !(create_info->db_type->flags & HTON_SUPPORT_LOG_TABLES)))
     {
@@ -9826,7 +9872,7 @@ bool mysql_alter_table(THD *thd, const LEX_CSTRING *new_db,
     if we can support implementing storage engine.
   */
   if (WSREP(thd) && table && table->s->sequence &&
-      wsrep_check_sequence(thd, thd->lex->create_info.seq_create_info))
+      wsrep_check_sequence(thd, thd->lex->create_info.seq_create_info, used_engine))
     DBUG_RETURN(TRUE);
 #endif /* WITH_WSREP */
 
@@ -10285,12 +10331,10 @@ do_continue:;
 #endif
 
 #ifdef WITH_WSREP
+  // ALTER TABLE for sequence object, check can we support it
   if (table->s->sequence && WSREP(thd) &&
-      wsrep_thd_is_local_toi(thd))
-  {
-    if (wsrep_check_sequence(thd, create_info->seq_create_info))
+      wsrep_check_sequence(thd, create_info->seq_create_info, used_engine))
       DBUG_RETURN(TRUE);
-  }
 #endif /* WITH_WSREP */
 
   /*
@@ -11744,17 +11788,11 @@ bool Sql_cmd_create_table_like::execute(THD *thd)
 #endif
 
 #ifdef WITH_WSREP
-  if (wsrep_ctas)
+  if (WSREP(thd) &&
+      wsrep_check_support(thd, wsrep_ctas, used_engine, &create_info))
   {
-    if (thd->variables.wsrep_trx_fragment_size > 0)
-    {
-      my_message(
-        ER_NOT_ALLOWED_COMMAND,
-        "CREATE TABLE AS SELECT is not supported with streaming replication",
-        MYF(0));
-      res= 1;
-      goto end_with_restore_list;
-    }
+    res= 1;
+    goto end_with_restore_list;
   }
 #endif /* WITH_WSREP */
 
@@ -11906,6 +11944,7 @@ bool Sql_cmd_create_table_like::execute(THD *thd)
                                    create_table->table_name, create_table->db))
 	goto end_with_restore_list;
 
+#ifdef WITH_WSREP
       /*
         In STATEMENT format, we probably have to replicate also temporary
         tables, like mysql replication does. Also check if the requested
@@ -11914,15 +11953,15 @@ bool Sql_cmd_create_table_like::execute(THD *thd)
       if (WSREP(thd))
       {
         handlerton *orig_ht= create_info.db_type;
+
         if (!check_engine(thd, create_table->db.str,
                           create_table->table_name.str,
                           &create_info) &&
             (!thd->is_current_stmt_binlog_format_row() ||
              !create_info.tmp_table()))
         {
-#ifdef WITH_WSREP
           if (thd->lex->sql_command == SQLCOM_CREATE_SEQUENCE &&
-              wsrep_check_sequence(thd, lex->create_info.seq_create_info))
+              wsrep_check_sequence(thd, lex->create_info.seq_create_info, used_engine))
             DBUG_RETURN(true);
 
           WSREP_TO_ISOLATION_BEGIN_ALTER(create_table->db.str, create_table->table_name.str,
@@ -11932,13 +11971,14 @@ bool Sql_cmd_create_table_like::execute(THD *thd)
             res= true;
             goto end_with_restore_list;
           }
-#endif /* WITH_WSREP */
         }
         // check_engine will set db_type to  NULL if e.g. TEMPORARY is
         // not supported by the storage engine, this case is checked
         // again in mysql_create_table
         create_info.db_type= orig_ht;
       }
+#endif /* WITH_WSREP */
+
       /* Regular CREATE TABLE */
       res= mysql_create_table(thd, create_table, &create_info, &alter_info);
     }

sql/sql_table.h

@@ -285,8 +285,4 @@ extern mysql_mutex_t LOCK_gdl;
 
 bool check_engine(THD *, const char *, const char *, HA_CREATE_INFO *);
 
-#ifdef WITH_WSREP
-bool wsrep_check_sequence(THD* thd, const class sequence_definition *seq);
-#endif
-
 #endif /* SQL_TABLE_INCLUDED */