MDEV-33011 mariabackup --backup: FATAL ERROR: ... Can't open datafile…

… cool_down/t3

The root cause is the WAL logging of file operation when the actual
operation fails afterwards. It creates a situation with a log entry for
a operation that would always fail. I could simulate both the backup
scenario error and Innodb recovery failure exploiting the weakness.

We are following WAL for file rename operation and once logged the
operation must eventually complete successfully, or it is a major
catastrophe. Right now, we fail for rename and handle it as normal error
and it is the problem.

I created a patch to address RENAME operation to a non existing schema
where the destination schema directory is missing. The patch checks for
the missing schema before logging in an attempt to avoid the failure
after WAL log is written/flushed. I also checked that the schema cannot
be dropped or there cannot be any race with other rename to the same
file. This is protected by the MDL lock in SQL today.

The patch should this be a good improvement over the current situation
and solves the issue at hand.

mysql-test/suite/innodb/r/rename_table.result

@@ -20,9 +20,15 @@ path
 DROP DATABASE abc_def;
 # restart
 DROP DATABASE abc_def2;
-call mtr.add_suppression("InnoDB: (Operating system error|The error means|Cannot rename file)");
+call mtr.add_suppression("InnoDB: Cannot rename '.*t1.ibd' to '.*non_existing_db.*' because the target schema directory doesn't exist");
 CREATE TABLE t1 (a INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES(100);
 RENAME TABLE t1 TO non_existing_db.t1;
 ERROR HY000: Error on rename of './test/t1' to './non_existing_db/t1' (errno: 168 "Unknown (generic) error from engine")
-FOUND 1 /\[ERROR\] InnoDB: Cannot rename file '.*t1\.ibd' to '.*non_existing_db/ in mysqld.1.err
+FOUND 1 /\[ERROR\] InnoDB: Cannot rename '.*t1\.ibd' to '.*non_existing_db/ in mysqld.1.err
+SET GLOBAL innodb_fast_shutdown=2;
+# restart
+SELECT * FROM t1;
+a
+100
 DROP TABLE t1;

mysql-test/suite/innodb/t/rename_table.test

@@ -30,16 +30,21 @@ DROP DATABASE abc_def;
 
 DROP DATABASE abc_def2;
 
-call mtr.add_suppression("InnoDB: (Operating system error|The error means|Cannot rename file)");
+call mtr.add_suppression("InnoDB: Cannot rename '.*t1.ibd' to '.*non_existing_db.*' because the target schema directory doesn't exist");
 
 CREATE TABLE t1 (a INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES(100);
 --replace_result "\\" "/"
 --error ER_ERROR_ON_RENAME
 RENAME TABLE t1 TO non_existing_db.t1;
 
---let SEARCH_PATTERN= \[ERROR\] InnoDB: Cannot rename file '.*t1\.ibd' to '.*non_existing_db
+--let SEARCH_PATTERN= \[ERROR\] InnoDB: Cannot rename '.*t1\.ibd' to '.*non_existing_db
 let SEARCH_FILE= $MYSQLTEST_VARDIR/log/mysqld.1.err;
 --source include/search_pattern_in_file.inc
 
+SET GLOBAL innodb_fast_shutdown=2;
+--source include/restart_mysqld.inc
+
+SELECT * FROM t1;
 # Cleanup
 DROP TABLE t1;

mysql-test/suite/mariabackup/rename_during_backup.result

@@ -60,3 +60,15 @@ SELECT * from t6;
 i
 5
 DROP TABLE t6;
+#
+# MDEV-33011 mariabackup --backup: FATAL ERROR: ... Can't open datafile cool_down/t3
+#
+# Simulate zero initialized page to defer tablespace load after rename log is found
+SET @save_dbug = @@SESSION.debug_dbug;
+SET DEBUG_DBUG="+d,checkpoint_after_file_create";
+CREATE TABLE t1(f1 INT NOT NULL)ENGINE=InnoDB;
+INSERT INTO t1 VALUES(1);
+# RENAME that fails after redo log entry is written and flushed
+RENAME TABLE t1 TO non_existing_db.t1;
+ERROR HY000: Error on rename of './test/t1' to './non_existing_db/t1' (errno: 168 "Unknown (generic) error from engine")
+DROP TABLE t1;

mysql-test/suite/mariabackup/rename_during_backup.test

@@ -90,4 +90,31 @@ SELECT * from t6;
 DROP TABLE t6;
 rmdir $targetdir;
 
+--echo #
+--echo # MDEV-33011 mariabackup --backup: FATAL ERROR: ... Can't open datafile cool_down/t3
+--echo #
 
+--disable_query_log
+call mtr.add_suppression("InnoDB: Cannot rename '.*t1.ibd' to '.*non_existing_db.*' because the target schema directory doesn't exist");
+--enable_query_log
+
+mkdir $targetdir;
+
+--echo # Simulate zero initialized page to defer tablespace load after rename log is found
+SET @save_dbug = @@SESSION.debug_dbug;
+SET DEBUG_DBUG="+d,checkpoint_after_file_create";
+CREATE TABLE t1(f1 INT NOT NULL)ENGINE=InnoDB;
+INSERT INTO t1 VALUES(1);
+
+--echo # RENAME that fails after redo log entry is written and flushed
+--replace_result "\\" "/"
+--error ER_ERROR_ON_RENAME
+RENAME TABLE t1 TO non_existing_db.t1;
+
+--disable_result_log
+exec $XTRABACKUP --defaults-file=$MYSQLTEST_VARDIR/my.cnf --backup --target-dir=$targetdir;
+exec $XTRABACKUP --prepare --target-dir=$targetdir;
+--enable_result_log
+
+DROP TABLE t1;
+rmdir $targetdir;

storage/innobase/fil/fil0fil.cc

@@ -1959,33 +1959,29 @@ Allocates and builds a file name from a path, a table or tablespace name
 and a suffix. The string must be freed by caller with ut_free().
 @param[in] path NULL or the directory path or the full path and filename.
 @param[in] name NULL if path is full, or Table/Tablespace name
-@param[in] suffix NULL or the file extention to use.
+@param[in] extension NULL or the file extension to use.
 @param[in] trim_name true if the last name on the path should be trimmed.
 @return own: file name */
 char*
-fil_make_filepath(
+fil_make_filepath_low(
 	const char*	path,
 	const char*	name,
-	ib_extention	ext,
+	ib_extention	extension,
 	bool		trim_name)
 {
 	/* The path may contain the basename of the file, if so we do not
 	need the name.  If the path is NULL, we can use the default path,
 	but there needs to be a name. */
 	ut_ad(path != NULL || name != NULL);
 
-	/* If we are going to strip a name off the path, there better be a
-	path and a new name to put back on. */
-	ut_ad(!trim_name || (path != NULL && name != NULL));
-
 	if (path == NULL) {
 		path = fil_path_to_mysql_datadir;
 	}
 
 	ulint	len		= 0;	/* current length */
 	ulint	path_len	= strlen(path);
 	ulint	name_len	= (name ? strlen(name) : 0);
-	const char* suffix	= dot_ext[ext];
+	const char* suffix	= dot_ext[extension];
 	ulint	suffix_len	= strlen(suffix);
 	ulint	full_len	= path_len + 1 + name_len + suffix_len + 1;
 
@@ -2083,6 +2079,21 @@ fil_rename_tablespace_check(
 	}
 
 	exists = false;
+	auto schema_path= fil_make_dirpath(new_path, NULL, NO_EXT, true);
+	if (schema_path == NULL) {
+		return DB_ERROR;
+	}
+
+	if (os_file_status(schema_path, &exists, &ftype) && !exists) {
+		sql_print_error("InnoDB: Cannot rename '%s' to '%s'"
+				" because the target schema directory doesn't exist.",
+				old_path, new_path);
+		ut_free(schema_path);
+		return DB_ERROR;
+	}
+	ut_free(schema_path);
+	exists = false;
+
 	if (os_file_status(new_path, &exists, &ftype) && !exists) {
 		return DB_SUCCESS;
 	}

storage/innobase/include/fil0fil.h

@@ -1618,14 +1618,44 @@ Allocates and builds a file name from a path, a table or tablespace name
 and a suffix. The string must be freed by caller with ut_free().
 @param[in] path NULL or the directory path or the full path and filename.
 @param[in] name NULL if path is full, or Table/Tablespace name
-@param[in] suffix NULL or the file extention to use.
+@param[in] extension NULL or the file extension to use.
+@param[in] trim_name true if the last name on the path should be trimmed.
 @return own: file name */
 char*
-fil_make_filepath(
+fil_make_filepath_low(
 	const char*	path,
 	const char*	name,
-	ib_extention	suffix,
-	bool		strip_name);
+	ib_extention	extension,
+	bool		trim_name);
+
+/** Wrapper function over fil_make_filepath_low to build file name.
+@param path NULL or the directory path or the full path and filename.
+@param name NULL if path is full, or Table/Tablespace name
+@param extension NULL or the file extension to use.
+@param trim_name true if the last name on the path should be trimmed.
+@return own: file name */
+static inline char*
+fil_make_filepath(const char* path, const char* name, ib_extention extension,
+                  bool trim_name)
+{
+  /* If we are going to strip a name off the path, there better be a
+  path and a new name to put back on. */
+  ut_ad(!trim_name || (path && name));
+  return fil_make_filepath_low(path, name, extension, trim_name);
+}
+
+/** Wrapper function over fil_make_filepath_low to build directory name.
+@param path NULL or the directory path or the full path and filename.
+@param name NULL if path is full, or Table/Tablespace name
+@param extension NULL or the file extension to use.
+@param trim_name true if the last name on the path should be trimmed.
+@return own: directory name */
+static inline char*
+fil_make_dirpath(const char* path, const char* name, ib_extention extension,
+                 bool trim_name)
+{
+  return fil_make_filepath_low(path, name, extension, trim_name);
+}
 
 /** Create a tablespace file.
 @param[in]	space_id	Tablespace ID