Fix that BACKUP STAGE BLOCK_COMMIT blocks commit to the Aria engine

MDEV-22468 BACKUP STAGE BLOCK_COMMIT should block commit in the Aria engine

This is needed to ensure that mariabackup works properly with Aria tables

This code ads new calls to ha_maria::implicit_commit(). These will be
deleted by MDEV-22531 Remove maria::implicit_commit().

mysql-test/main/backup_lock.result

@@ -192,8 +192,10 @@ BACKUP STAGE END;
 SET GLOBAL lock_wait_timeout=0;
 CREATE TABLE t_permanent_innodb (col1 INT) ENGINE = InnoDB;
 CREATE TABLE t_permanent_myisam (col1 INT) ENGINE = MyISAM;
+CREATE TABLE t_permanent_aria  (col1 INT) ENGINE = Aria transactional=1;
 INSERT INTO t_permanent_innodb SET col1 = 1;
 INSERT INTO t_permanent_myisam SET col1 = 1;
+INSERT INTO t_permanent_aria SET col1 = 1;
 CREATE TABLE t_con1_innodb (col1 INT) ENGINE = InnoDB;
 CREATE TABLE t_con1_myisam (col1 INT) ENGINE = MyISAM;
 connect con1,localhost,root,,;
@@ -207,13 +209,23 @@ connection con1;
 UPDATE t_permanent_innodb SET col1 = 8;
 UPDATE t_permanent_myisam SET col1 = 8;
 ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+UPDATE t_permanent_aria SET col1 = 8;
 DROP TABLE t_con1_innodb;
 ERROR HY000: Lock wait timeout exceeded; try restarting transaction
 DROP TABLE t_con1_myisam;
 ERROR HY000: Lock wait timeout exceeded; try restarting transaction
 connection default;
 BACKUP STAGE END;
-DROP TABLE t_permanent_myisam, t_permanent_innodb;
+select * from t_permanent_innodb;
+col1
+1
+select * from t_permanent_myisam;
+col1
+1
+select * from t_permanent_aria;
+col1
+8
+DROP TABLE t_permanent_myisam, t_permanent_innodb, t_permanent_aria;
 DROP TABLE t_con1_innodb, t_con1_myisam;
 disconnect con1;
 set global lock_wait_timeout=default;

mysql-test/main/backup_lock.test

@@ -251,9 +251,11 @@ BACKUP STAGE END;
 SET GLOBAL lock_wait_timeout=0;
 CREATE TABLE t_permanent_innodb (col1 INT) ENGINE = InnoDB;
 CREATE TABLE t_permanent_myisam (col1 INT) ENGINE = MyISAM;
+CREATE TABLE t_permanent_aria  (col1 INT) ENGINE = Aria transactional=1;
 INSERT INTO t_permanent_innodb SET col1 = 1;
-
 INSERT INTO t_permanent_myisam SET col1 = 1;
+INSERT INTO t_permanent_aria SET col1 = 1;
+
 CREATE TABLE t_con1_innodb (col1 INT) ENGINE = InnoDB;
 CREATE TABLE t_con1_myisam (col1 INT) ENGINE = MyISAM;
 
@@ -270,6 +272,8 @@ BACKUP STAGE BLOCK_COMMIT;
 UPDATE t_permanent_innodb SET col1 = 8;
 --error ER_LOCK_WAIT_TIMEOUT
 UPDATE t_permanent_myisam SET col1 = 8;
+UPDATE t_permanent_aria SET col1 = 8;
+
 --error ER_LOCK_WAIT_TIMEOUT
 DROP TABLE t_con1_innodb;
 
@@ -278,7 +282,12 @@ DROP TABLE t_con1_myisam;
 
 --connection default
 BACKUP STAGE END;
-DROP TABLE t_permanent_myisam, t_permanent_innodb;
+
+select * from t_permanent_innodb;
+select * from t_permanent_myisam;
+select * from t_permanent_aria;
+
+DROP TABLE t_permanent_myisam, t_permanent_innodb, t_permanent_aria;
 DROP TABLE t_con1_innodb, t_con1_myisam;
 --disconnect con1
 set global lock_wait_timeout=default;

sql/handler.cc

@@ -1531,16 +1531,6 @@ int ha_commit_trans(THD *thd, bool all)
     DBUG_RETURN(2);
   }
 
-#ifdef WITH_ARIA_STORAGE_ENGINE
-  if ((error= ha_maria::implicit_commit(thd, TRUE)))
-  {
-    my_error(ER_ERROR_DURING_COMMIT, MYF(0), error);
-    ha_rollback_trans(thd, all);
-    DBUG_RETURN(1);
-  }
-
-#endif
-
   if (!ha_info)
   {
     /*
@@ -1556,6 +1546,16 @@ int ha_commit_trans(THD *thd, bool all)
     if (wsrep_is_active(thd) && is_real_trans && !error)
       wsrep_commit_empty(thd, all);
 #endif /* WITH_WSREP */
+
+#if defined(WITH_ARIA_STORAGE_ENGINE)
+    /* This is needed to ensure that repair commits properly */
+    if ((error= ha_maria::implicit_commit(thd, TRUE)))
+    {
+      my_error(ER_ERROR_DURING_COMMIT, MYF(0), error);
+      ha_rollback_trans(thd, all);
+      DBUG_RETURN(1);
+    }
+#endif
     DBUG_RETURN(0);
   }
 
@@ -1570,10 +1570,16 @@ int ha_commit_trans(THD *thd, bool all)
   bool rw_trans= is_real_trans &&
                  (rw_ha_count > (thd->is_current_stmt_binlog_disabled()?0U:1U));
   MDL_request mdl_request;
+  mdl_request.ticket= 0;
   DBUG_PRINT("info", ("is_real_trans: %d  rw_trans:  %d  rw_ha_count: %d",
                       is_real_trans, rw_trans, rw_ha_count));
 
-  if (rw_trans)
+  /*
+    We need to test maria_hton because of plugin_innodb.test that changes
+    the plugin table to innodb and thus plugin_load will call
+    mysql_close_tables() which calls trans_commit_trans() with maria_hton = 0
+  */
+  if (rw_trans || (likely(maria_hton) && thd_get_ha_data(thd, maria_hton)))
   {
     /*
       Acquire a metadata lock which will ensure that COMMIT is blocked
@@ -1587,8 +1593,8 @@ int ha_commit_trans(THD *thd, bool all)
                      MDL_EXPLICIT);
 
     if (!WSREP(thd) &&
-      thd->mdl_context.acquire_lock(&mdl_request,
-                                    thd->variables.lock_wait_timeout))
+        thd->mdl_context.acquire_lock(&mdl_request,
+                                      thd->variables.lock_wait_timeout))
     {
       ha_rollback_trans(thd, all);
       DBUG_RETURN(1);
@@ -1596,6 +1602,13 @@ int ha_commit_trans(THD *thd, bool all)
 
     DEBUG_SYNC(thd, "ha_commit_trans_after_acquire_commit_lock");
   }
+#if defined(WITH_ARIA_STORAGE_ENGINE)
+    if ((error= ha_maria::implicit_commit(thd, TRUE)))
+    {
+      my_error(ER_ERROR_DURING_COMMIT, MYF(0), error);
+      goto err;
+    }
+#endif
 
   if (rw_trans &&
       opt_readonly &&
@@ -1797,7 +1810,7 @@ int ha_commit_trans(THD *thd, bool all)
                 thd->rgi_slave->is_parallel_exec);
   }
 end:
-  if (rw_trans && mdl_request.ticket)
+  if (mdl_request.ticket)
   {
     /*
       We do not always immediately release transactional locks

sql/handler.h

@@ -70,6 +70,7 @@ class Column_definition;
 #define HA_ADMIN_NEEDS_UPGRADE  -10
 #define HA_ADMIN_NEEDS_ALTER    -11
 #define HA_ADMIN_NEEDS_CHECK    -12
+#define HA_ADMIN_COMMIT_ERROR   -13
 
 /**
    Return values for check_if_supported_inplace_alter().

sql/sql_parse.cc

@@ -1508,6 +1508,67 @@ uint maria_multi_check(THD *thd, char *packet, size_t packet_length)
 }
 
 
+#if defined(WITH_ARIA_STORAGE_ENGINE)
+class Silence_all_errors : public Internal_error_handler
+{
+  char m_message[MYSQL_ERRMSG_SIZE];
+  int error;
+public:
+  Silence_all_errors():error(0) {}
+  virtual ~Silence_all_errors() {}
+
+  virtual bool handle_condition(THD *thd,
+                                uint sql_errno,
+                                const char* sql_state,
+                                Sql_condition::enum_warning_level *level,
+                                const char* msg,
+                                Sql_condition ** cond_hdl)
+  {
+    error= sql_errno;
+    *cond_hdl= NULL;
+    strmake_buf(m_message, msg);
+    return true;                              // Error handled
+  }
+};
+#endif
+
+/*
+  Do an implict commit into the Aria storage engine
+*/
+
+static inline my_bool aria_implicit_commit(THD *thd)
+{
+#if defined(WITH_ARIA_STORAGE_ENGINE)
+  if (thd_get_ha_data(thd, maria_hton))
+  {
+    MDL_request mdl_request;
+    bool locked;
+    int res;
+    Silence_all_errors error_handler;
+    DBUG_ASSERT(maria_hton);
+
+    MDL_REQUEST_INIT(&mdl_request, MDL_key::BACKUP, "", "", MDL_BACKUP_COMMIT,
+                     MDL_EXPLICIT);
+    /*
+      We have to ignore any errors from acquire_lock and continue even if we
+      don't get the lock as Aria can't roll back!
+      This function is also called in some cases when the message is already
+      sent to the user, so we can't even send a warning.
+  */
+    thd->push_internal_handler(& error_handler);
+    locked= !thd->mdl_context.acquire_lock(&mdl_request,
+                                           thd->variables.lock_wait_timeout);
+    thd->pop_internal_handler();
+    res= ha_maria::implicit_commit(thd, FALSE);
+    if (locked)
+      thd->mdl_context.release_lock(mdl_request.ticket);
+    return res;
+  }
+#endif
+  return 0;
+}
+
+
 /**
   Perform one connection-level (COM_XXXX) command.
 
@@ -1860,9 +1921,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
       */
       char *beginning_of_next_stmt= (char*) parser_state.m_lip.found_semicolon;
 
-#ifdef WITH_ARIA_STORAGE_ENGINE
-    ha_maria::implicit_commit(thd, FALSE);
-#endif
+      aria_implicit_commit(thd);
 
       /* Finalize server status flags after executing a statement. */
       thd->update_server_status();
@@ -5990,9 +6049,7 @@ mysql_execute_command(THD *thd)
       trans_commit_stmt(thd);
       thd->get_stmt_da()->set_overwrite_status(false);
     }
-#ifdef WITH_ARIA_STORAGE_ENGINE
-    ha_maria::implicit_commit(thd, FALSE);
-#endif
+    aria_implicit_commit(thd);
   }
 
   /* Free tables. Set stage 'closing tables' */

storage/maria/ha_maria.cc

@@ -1477,6 +1477,13 @@ int ha_maria::repair(THD * thd, HA_CHECK_OPT *check_opt)
     }
     break;
   }
+  /*
+    Commit is needed in the case of tables are locked to ensure that repair
+    is registered in the recovery log
+  */
+  if (implicit_commit(thd, TRUE))
+    error= HA_ADMIN_COMMIT_ERROR;
+
   if (!error && start_records != file->state->records &&
       !(check_opt->flags & T_VERY_SILENT))
   {