MDEV-25854: Remove garbage tables after restoring a backup

In commit 1c5ae99194 (MDEV-25666)
we had changed Mariabackup so that it would no longer skip files
whose names start with #sql. This turned out to be wrong.
Because operations on such named files are not protected by any
locks in the server, it is not safe to copy them.

Not copying the files may make the InnoDB data dictionary
inconsistent with the file system. So, we must do something
in InnoDB to adjust for that.

If InnoDB is being started up without the redo log (ib_logfile0)
or with a zero-length log file, we will assume that the server
was restored from a backup, and adjust things as follows:

dict_check_sys_tables(), fil_ibd_open(): Do not complain about
missing #sql files if they would be dropped a little later.

dict_stats_update_if_needed(): Never add #sql tables to
the recomputing queue. This avoids a potential race condition when
dropping the garbage tables.

drop_garbage_tables_after_restore(): Try to drop any garbage tables.

innodb_ddl_recovery_done(): Invoke drop_garbage_tables_after_restore()
if srv_start_after_restore (a new flag) was set and we are not in
read-only mode (innodb_read_only=ON or innodb_force_recovery>3).

The tests and dbug_mariabackup_event() instrumentation
were developed by Vladislav Vaintroub, who also reviewed this.

extra/mariabackup/backup_mysql.cc

@@ -921,9 +921,11 @@ bool lock_tables(MYSQL *connection)
   }
 
   xb_mysql_query(connection, "BACKUP STAGE START", true);
+  DBUG_MARIABACKUP_EVENT("after_backup_stage_start", {});
   // xb_mysql_query(connection, "BACKUP STAGE FLUSH", true);
   // xb_mysql_query(connection, "BACKUP STAGE BLOCK_DDL", true);
   xb_mysql_query(connection, "BACKUP STAGE BLOCK_COMMIT", true);
+  DBUG_MARIABACKUP_EVENT("after_backup_stage_block_commit", {});
   /* Set the maximum supported session value for
   lock_wait_timeout to prevent unnecessary timeouts when the
   global value is changed from the default */

extra/mariabackup/xtrabackup.cc

@@ -2546,11 +2546,24 @@ check_if_skip_table(
 
 	dbname = NULL;
 	tbname = name;
-	while ((ptr = strchr(tbname, '/')) != NULL) {
+	for (;;) {
+		ptr= strchr(tbname, '/');
+#ifdef _WIN32
+		if (!ptr) {
+			ptr= strchr(tbname,'\\');
+		}
+#endif
+		if (!ptr) {
+			break;
+		}
 		dbname = tbname;
 		tbname = ptr + 1;
 	}
 
+	if (strncmp(tbname, tmp_file_prefix, tmp_file_prefix_length) == 0) {
+		return TRUE;
+	}
+
 	if (regex_exclude_list.empty() &&
 		regex_include_list.empty() &&
 		!tables_include_hash.array &&
@@ -3038,7 +3051,7 @@ To use this facility, you need to
    for the variable)
 3. start mariabackup with --dbug=+d,debug_mariabackup_events
 */
-static void dbug_mariabackup_event(const char *event,
+void dbug_mariabackup_event(const char *event,
                                    const fil_space_t::name_type key)
 {
 	char *sql = dbug_mariabackup_get_val(event, key);
@@ -3047,10 +3060,6 @@ static void dbug_mariabackup_event(const char *event,
 		xb_mysql_query(mysql_connection, sql, false, true);
 	}
 }
-# define DBUG_MARIABACKUP_EVENT(A, B)					\
-  DBUG_EXECUTE_IF("mariabackup_events", dbug_mariabackup_event(A,B);)
-#else
-# define DBUG_MARIABACKUP_EVENT(A, B) /* empty */
 #endif // DBUG_OFF
 
 /** Datafiles copying thread.*/

extra/mariabackup/xtrabackup.h

@@ -284,4 +284,16 @@ fil_file_readdir_next_file(
 	os_file_dir_t	dir,	/*!< in: directory stream */
 	os_file_stat_t* info);	/*!< in/out: buffer where the
 				info is returned */
+
+#ifndef DBUG_OFF
+#include <fil0fil.h>
+extern void dbug_mariabackup_event(const char *event,
+                            const fil_space_t::name_type key);
+
+#define DBUG_MARIABACKUP_EVENT(A, B)                                          \
+  DBUG_EXECUTE_IF("mariabackup_events", dbug_mariabackup_event(A, B);)
+#else
+#define DBUG_MARIABACKUP_EVENT(A, B) /* empty */
+#endif // DBUG_OFF
+
 #endif /* XB_XTRABACKUP_H */

mysql-test/suite/mariabackup/alter_copy_excluded.opt

@@ -0,0 +1 @@
+--loose-innodb_sys_tablespaces

mysql-test/suite/mariabackup/alter_copy_excluded.result

@@ -0,0 +1,25 @@
+# xtrabackup backup
+CREATE TABLE t1(i int) ENGINE=InnoDB;
+INSERT into t1 values(1);
+connect  con2, localhost, root,,;
+connection con2;
+SET debug_sync='copy_data_between_tables_before_reset_backup_lock SIGNAL go  WAIT_FOR after_backup_stage_block_commit' ;
+SET debug_sync='now WAIT_FOR after_backup_stage_start';ALTER TABLE test.t1 FORCE, algorithm=COPY;|
+connection default;
+connection con2;
+SET debug_sync='RESET';
+disconnect con2;
+connection default;
+# xtrabackup prepare
+# shutdown server
+# remove datadir
+# xtrabackup move back
+# restart
+SELECT COUNT(*) AS expect_0 FROM INFORMATION_SCHEMA.innodb_sys_tablespaces WHERE name like '%/#sql%';
+expect_0
+0
+SELECT * FROM t1;
+i
+1
+DROP TABLE t1;
+# restart

mysql-test/suite/mariabackup/alter_copy_excluded.test

@@ -0,0 +1,65 @@
+--source include/have_innodb.inc
+--source include/have_debug.inc
+
+# The test demonstrates that intermediate tables (ALTER TABLE...ALGORITHM=COPY)
+# will not be included in a backup.
+
+echo # xtrabackup backup;
+let $targetdir=$MYSQLTEST_VARDIR/tmp/backup;
+
+CREATE TABLE t1(i int) ENGINE=InnoDB;
+INSERT into t1 values(1);
+
+connect  con2, localhost, root,,;
+connection con2;
+SET debug_sync='copy_data_between_tables_before_reset_backup_lock SIGNAL go  WAIT_FOR after_backup_stage_block_commit' ;
+DELIMITER |;
+send  SET debug_sync='now WAIT_FOR after_backup_stage_start';ALTER TABLE test.t1 FORCE, algorithm=COPY;|
+DELIMITER ;|
+connection default;
+
+# Setup mariabackup events
+# - After BACKUP STAGE START , let concurrent ALTER run, wand wait for it to  create temporary tables
+# - After BACKUP STAGE COMMIT, check that temporary files are in the database
+
+let after_backup_stage_start=SET debug_sync='now SIGNAL after_backup_stage_start WAIT_FOR go';
+DELIMITER |;
+# The following query only works if there are innodb "intermediate" tables
+# in the system tables , which we want to prove there
+let after_backup_stage_block_commit=
+  IF (SELECT COUNT(*) > 0 FROM INFORMATION_SCHEMA.innodb_sys_tablespaces WHERE name like '%/#sql%') THEN
+    SET debug_sync='now SIGNAL after_backup_stage_block_commit';
+  END IF|
+DELIMITER ;|
+
+--disable_result_log
+exec  $XTRABACKUP --defaults-file=$MYSQLTEST_VARDIR/my.cnf  --backup  --target-dir=$targetdir  --dbug=+d,mariabackup_events;
+--enable_result_log
+
+# There should be no temp files in the backup.
+--list_files $targetdir/test #sql*
+
+connection con2;
+#Wait for ALTER to finish, cleanup
+reap;
+SET debug_sync='RESET';
+disconnect con2;
+
+connection default;
+echo # xtrabackup prepare;
+--disable_result_log
+exec $XTRABACKUP  --prepare --target-dir=$targetdir;
+-- source include/restart_and_restore.inc
+--enable_result_log
+
+# Check there are no temp tablespaces in sys_tablespaces, after backup
+SELECT COUNT(*) AS expect_0 FROM INFORMATION_SCHEMA.innodb_sys_tablespaces WHERE name like '%/#sql%';
+SELECT * FROM t1;
+DROP TABLE t1;
+
+# Restart once again to clear first_start_after_backup flag
+# This is to catch potential warnings, since "missing file" for #sql is suppressed
+# during the first start after backup
+--source include/restart_mysqld.inc
+
+rmdir $targetdir;

mysql-test/suite/mariabackup/alter_copy_race.result

@@ -0,0 +1,24 @@
+# xtrabackup backup
+CREATE TABLE t1(i int) ENGINE=InnoDB;
+INSERT into t1 values(1);
+connect  con2, localhost, root,,;
+connection con2;
+set lock_wait_timeout=1;
+SET debug_sync='copy_data_between_tables_before_reset_backup_lock  SIGNAL go WAIT_FOR after_backup_stage_block_commit';
+SET debug_sync='alter_table_after_temp_table_drop SIGNAL temp_table_dropped';
+SET debug_sync='now WAIT_FOR after_backup_stage_start';ALTER TABLE test.t1 FORCE, algorithm=COPY;|
+connection default;
+connection con2;
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+SET debug_sync='RESET';
+disconnect con2;
+connection default;
+# xtrabackup prepare
+# shutdown server
+# remove datadir
+# xtrabackup move back
+# restart
+SELECT * FROM t1;
+i
+1
+DROP TABLE t1;

mysql-test/suite/mariabackup/alter_copy_race.test

@@ -0,0 +1,50 @@
+--source include/have_innodb.inc
+--source include/have_debug.inc
+
+# The test demonstrates that intermediate tables (ALTER TABLE...ALGORITHM=COPY)
+# are not always properly locked, e.g., can be dropped after
+#  BACKUP STAGE BLOCK_COMMIT
+# succeeded.
+# Thus mariabackup decides not to have them in backup at all,
+# since they keep changing even after the backup LSN was determined.
+
+echo # xtrabackup backup;
+let $targetdir=$MYSQLTEST_VARDIR/tmp/backup;
+
+CREATE TABLE t1(i int) ENGINE=InnoDB;
+INSERT into t1 values(1);
+
+connect  con2, localhost, root,,;
+connection con2;
+set lock_wait_timeout=1;
+SET debug_sync='copy_data_between_tables_before_reset_backup_lock  SIGNAL go WAIT_FOR after_backup_stage_block_commit';
+SET debug_sync='alter_table_after_temp_table_drop SIGNAL temp_table_dropped';
+DELIMITER |;
+send  SET debug_sync='now WAIT_FOR after_backup_stage_start';ALTER TABLE test.t1 FORCE, algorithm=COPY;|
+DELIMITER ;|
+connection default;
+
+# setup mariabackup events
+let after_backup_stage_start=SET debug_sync='now SIGNAL after_backup_stage_start WAIT_FOR go';
+let after_backup_stage_block_commit=SET debug_sync='now SIGNAL after_backup_stage_block_commit';
+let backup_fix_ddl=SET debug_sync='now WAIT_FOR temp_table_dropped';
+--disable_result_log
+exec  $XTRABACKUP --defaults-file=$MYSQLTEST_VARDIR/my.cnf  --backup  --target-dir=$targetdir  --dbug=+d,mariabackup_events;
+--enable_result_log
+
+connection con2;
+--error ER_LOCK_WAIT_TIMEOUT
+reap;
+SET debug_sync='RESET';
+disconnect con2;
+
+connection default;
+echo # xtrabackup prepare;
+--disable_result_log
+exec $XTRABACKUP  --prepare --target-dir=$targetdir;
+-- source include/restart_and_restore.inc
+--enable_result_log
+
+SELECT * FROM t1;
+DROP TABLE t1;
+rmdir $targetdir;

sql/sql_table.cc

@@ -10743,7 +10743,7 @@ err_new_table_cleanup:
                           &alter_ctx.new_db, &alter_ctx.tmp_name,
                           (FN_IS_TMP | (no_ha_table ? NO_HA_TABLE : 0)),
                           alter_ctx.get_tmp_path());
-
+  DEBUG_SYNC(thd, "alter_table_after_temp_table_drop");
 err_cleanup:
   my_free(const_cast<uchar*>(frm.str));
   ddl_log_complete(&ddl_log_state);
@@ -11178,6 +11178,7 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
   cleanup_done= 1;
   to->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
 
+  DEBUG_SYNC(thd, "copy_data_between_tables_before_reset_backup_lock");
   if (backup_reset_alter_copy_lock(thd))
     error= 1;
 

storage/innobase/dict/dict0load.cc

@@ -888,11 +888,18 @@ static ulint dict_check_sys_tables()
 						     IBD, false);
 
 		/* Check that the .ibd file exists. */
-		if (!fil_ibd_open(
-			    false,
-			    FIL_TYPE_TABLESPACE,
-			    space_id, dict_tf_to_fsp_flags(flags),
-			    name, filepath)) {
+		if (fil_ibd_open(false, FIL_TYPE_TABLESPACE,
+				 space_id, dict_tf_to_fsp_flags(flags),
+				 name, filepath)) {
+		} else if (srv_operation == SRV_OPERATION_NORMAL
+			   && srv_start_after_restore
+			   && srv_force_recovery < SRV_FORCE_NO_BACKGROUND
+			   && dict_table_t::is_temporary_name(filepath)) {
+			/* Mariabackup will not copy files whose
+			names start with #sql-. This table ought to
+			be dropped by drop_garbage_tables_after_restore()
+			a little later. */
+		} else {
 			sql_print_warning("InnoDB: Ignoring tablespace for"
 					  " %.*s because it"
 					  " could not be opened.",

storage/innobase/dict/dict0stats_bg.cc

@@ -168,6 +168,9 @@ void dict_stats_update_if_needed_func(dict_table_t *table)
 	ulonglong	n_rows = dict_table_get_n_rows(table);
 
 	if (dict_stats_is_persistent_enabled(table)) {
+		if (table->name.is_temporary()) {
+			return;
+		}
 		if (counter > n_rows / 10 /* 10% */
 		    && dict_stats_auto_recalc_is_enabled(table)) {
 

storage/innobase/fil/fil0fil.cc

@@ -2246,7 +2246,19 @@ func_exit:
 	/* Always look for a file at the default location. But don't log
 	an error if the tablespace is already open in remote or dict. */
 	ut_a(df_default.filepath());
-	const bool	strict = (tablespaces_found == 0);
+
+	/* Mariabackup will not copy files whose names start with
+	#sql-. We will suppress messages about such files missing on
+	the first server startup. The tables ought to be dropped by
+	drop_garbage_tables_after_restore() a little later. */
+
+	const bool strict = !tablespaces_found
+		&& !(srv_operation == SRV_OPERATION_NORMAL
+		     && srv_start_after_restore
+		     && srv_force_recovery < SRV_FORCE_NO_BACKGROUND
+		     && dict_table_t::is_temporary_name(
+			     df_default.filepath()));
+
 	if (df_default.open_read_only(strict) == DB_SUCCESS) {
 		ut_ad(df_default.is_open());
 		++tablespaces_found;
@@ -2281,6 +2293,13 @@ func_exit:
 	/* Make sense of these three possible locations.
 	First, bail out if no tablespace files were found. */
 	if (valid_tablespaces_found == 0) {
+		if (!strict
+		    && IF_WIN(GetLastError() == ERROR_FILE_NOT_FOUND,
+			      errno == ENOENT)) {
+			/* Suppress a message about a missing file. */
+			goto corrupted;
+		}
+
 		os_file_get_last_error(true);
 		sql_print_error("InnoDB: Could not find a valid tablespace"
 				" file for %.*s. %s",

storage/innobase/handler/ha_innodb.cc

@@ -1954,6 +1954,112 @@ static int innodb_check_version(handlerton *hton, const char *path,
     DBUG_RETURN(2);
 }
 
+/** Drop any garbage intermediate tables that existed in the system
+after a backup was restored.
+
+In a final phase of Mariabackup, the commit of DDL operations is blocked,
+and those DDL operations will have to be rolled back. Because the
+normal DDL recovery will not run due to the lack of the log file,
+at least some #sql-alter- garbage tables may remain in the InnoDB
+data dictionary (while the data files themselves are missing).
+We will attempt to drop the tables here. */
+static void drop_garbage_tables_after_restore()
+{
+  btr_pcur_t pcur;
+  mtr_t mtr;
+  trx_t *trx= trx_create();
+
+  mtr.start();
+  btr_pcur_open_at_index_side(true, dict_sys.sys_tables->indexes.start,
+                              BTR_SEARCH_LEAF, &pcur, true, 0, &mtr);
+  for (;;)
+  {
+    btr_pcur_move_to_next_user_rec(&pcur, &mtr);
+
+    if (!btr_pcur_is_on_user_rec(&pcur))
+      break;
+
+    const rec_t *rec= btr_pcur_get_rec(&pcur);
+    if (rec_get_deleted_flag(rec, 0))
+      continue;
+
+    static_assert(DICT_FLD__SYS_TABLES__NAME == 0, "compatibility");
+    size_t len;
+    if (rec_get_1byte_offs_flag(rec))
+    {
+      len= rec_1_get_field_end_info(rec, 0);
+      if (len & REC_1BYTE_SQL_NULL_MASK)
+        continue; /* corrupted SYS_TABLES.NAME */
+    }
+    else
+    {
+      len= rec_2_get_field_end_info(rec, 0);
+      static_assert(REC_2BYTE_EXTERN_MASK == 16384, "compatibility");
+      if (len >= REC_2BYTE_EXTERN_MASK)
+        continue; /* corrupted SYS_TABLES.NAME */
+    }
+
+    if (len < tmp_file_prefix_length)
+      continue;
+    if (const char *f= static_cast<const char*>
+        (memchr(rec, '/', len - tmp_file_prefix_length)))
+    {
+      if (memcmp(f + 1, tmp_file_prefix, tmp_file_prefix_length))
+        continue;
+    }
+    else
+      continue;
+
+    btr_pcur_store_position(&pcur, &mtr);
+    btr_pcur_commit_specify_mtr(&pcur, &mtr);
+
+    trx_start_for_ddl(trx);
+    std::vector<pfs_os_file_t> deleted;
+    row_mysql_lock_data_dictionary(trx);
+    dberr_t err= DB_TABLE_NOT_FOUND;
+
+    if (dict_table_t *table= dict_sys.load_table
+        ({reinterpret_cast<const char*>(pcur.old_rec), len},
+         DICT_ERR_IGNORE_DROP))
+    {
+      ut_ad(table->stats_bg_flag == BG_STAT_NONE);
+      table->acquire();
+      err= lock_table_for_trx(table, trx, LOCK_X);
+      if (err == DB_SUCCESS &&
+          (table->flags2 & (DICT_TF2_FTS_HAS_DOC_ID | DICT_TF2_FTS)))
+      {
+        fts_optimize_remove_table(table);
+        err= fts_lock_tables(trx, *table);
+      }
+      table->release();
+
+      if (err == DB_SUCCESS)
+        err= trx->drop_table(*table);
+      if (err != DB_SUCCESS)
+        goto fail;
+      trx->commit(deleted);
+    }
+    else
+    {
+fail:
+      trx->rollback();
+      sql_print_error("InnoDB: cannot drop %.*s: %s",
+                      static_cast<int>(len), pcur.old_rec, ut_strerr(err));
+    }
+
+    row_mysql_unlock_data_dictionary(trx);
+    for (pfs_os_file_t d : deleted)
+      os_file_close(d);
+
+    mtr.start();
+    btr_pcur_restore_position(BTR_SEARCH_LEAF, &pcur, &mtr);
+  }
+
+  btr_pcur_close(&pcur);
+  mtr.commit();
+  trx->free();
+}
+
 static void innodb_ddl_recovery_done(handlerton*)
 {
   ut_ad(!ddl_recovery_done);
@@ -1961,6 +2067,8 @@ static void innodb_ddl_recovery_done(handlerton*)
   if (!srv_read_only_mode && srv_operation == SRV_OPERATION_NORMAL &&
       srv_force_recovery < SRV_FORCE_NO_BACKGROUND)
   {
+    if (srv_start_after_restore && !high_level_read_only)
+      drop_garbage_tables_after_restore();
     srv_init_purge_tasks();
     purge_sys.coordinator_startup();
     srv_wake_purge_thread_if_not_active();

storage/innobase/include/srv0srv.h

@@ -430,6 +430,9 @@ enum srv_operation_mode {
 /** Current mode of operation */
 extern enum srv_operation_mode srv_operation;
 
+/** whether this is the server's first start after mariabackup --prepare */
+extern bool srv_start_after_restore;
+
 extern my_bool	srv_print_innodb_monitor;
 extern my_bool	srv_print_innodb_lock_monitor;
 extern ibool	srv_print_verbose_log;

storage/innobase/srv/srv0srv.cc

@@ -370,6 +370,9 @@ ulonglong	srv_defragment_interval;
 /** Current mode of operation */
 enum srv_operation_mode srv_operation;
 
+/** whether this is the server's first start after mariabackup --prepare */
+bool srv_start_after_restore;
+
 /* Set the following to 0 if you want InnoDB to write messages on
 stderr on startup/shutdown. Not enabled on the embedded server. */
 ibool	srv_print_verbose_log;

storage/innobase/srv/srv0start.cc

@@ -989,6 +989,9 @@ static dberr_t find_and_check_log_file(bool &log_file_found)
     if (is_operation_restore())
       return DB_NOT_FOUND;
 
+    /* This might be first start after mariabackup
+    copy-back or move-back. */
+    srv_start_after_restore= true;
     return DB_SUCCESS;
   }
 
@@ -1019,7 +1022,9 @@ static dberr_t find_and_check_log_file(bool &log_file_found)
   header, checkpoint page 1, empty, checkpoint page 2, redo log page(s).
 
   Mariabackup --prepare would create an empty LOG_FILE_NAME. Tolerate it. */
-  if (size != 0 && size <= OS_FILE_LOG_BLOCK_SIZE * 4)
+  if (size == 0)
+    srv_start_after_restore= true;
+  else if (size <= OS_FILE_LOG_BLOCK_SIZE * 4)
   {
     ib::error() << "Log file " << logfile0 << " size " << size
                 << " is too small";