MDEV-16124 fil_rename_tablespace() times out and crashes server during table-rebuilding ALTER TABLE

InnoDB insisted on closing the file handle before renaming a file.
Renaming a file should never be a problem on POSIX systems. Also on
Windows it should work if the file was opened in FILE_SHARE_DELETE
mode.

fil_space_t::stop_ios: Remove. We no longer need to stop file access
during rename operations.

fil_mutex_enter_and_prepare_for_io(): Remove the wait for stop_ios.

fil_rename_tablespace(): Remove the retry logic; do not close the
file handle. Remove the unused fault injection that was added along
with the DATA DIRECTORY functionality (MySQL WL#5980).

os_file_create_simple_func(), os_file_create_func(),
os_file_create_simple_no_error_handling_func(): Include FILE_SHARE_DELETE
in the share_mode. (We will still prevent multiple InnoDB instances
from using the same files by not setting FILE_SHARE_WRITE.)

6 files changed, 10 insertions, 236 deletions

diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc
index 7167b99a750..f852a64e2e9 100644
--- a/storage/innobase/fil/fil0fil.cc
+++ b/storage/innobase/fil/fil0fil.cc
@@ -845,7 +845,6 @@ fil_mutex_enter_and_prepare_for_io(
 	ibool		success;
 	ibool		print_info	= FALSE;
 	ulint		count		= 0;
-	ulint		count2		= 0;
 
 retry:
 	mutex_enter(&fil_system->mutex);
@@ -862,46 +861,6 @@ retry:
 
 	space = fil_space_get_by_id(space_id);
 
-	if (space != NULL && space->stop_ios) {
-		/* We are going to do a rename file and want to stop new i/o's
-		for a while */
-
-		if (count2 > 20000) {
-			fputs("InnoDB: Warning: tablespace ", stderr);
-			ut_print_filename(stderr, space->name);
-			fprintf(stderr,
-				" has i/o ops stopped for a long time %lu\n",
-				(ulong) count2);
-		}
-
-		mutex_exit(&fil_system->mutex);
-
-#ifndef UNIV_HOTBACKUP
-
-		/* Wake the i/o-handler threads to make sure pending
-		i/o's are performed */
-		os_aio_simulated_wake_handler_threads();
-
-		/* The sleep here is just to give IO helper threads a
-		bit of time to do some work. It is not required that
-		all IO related to the tablespace being renamed must
-		be flushed here as we do fil_flush() in
-		fil_rename_tablespace() as well. */
-		os_thread_sleep(20000);
-
-#endif /* UNIV_HOTBACKUP */
-
-		/* Flush tablespaces so that we can close modified
-		files in the LRU list */
-		fil_flush_file_spaces(FIL_TABLESPACE);
-
-		os_thread_sleep(20000);
-
-		count2++;
-
-		goto retry;
-	}
-
 	if (fil_system->n_open < fil_system->max_n_open) {
 
 		return;
@@ -2898,7 +2857,6 @@ fil_rename_tablespace(
 	ibool		success;
 	fil_space_t*	space;
 	fil_node_t*	node;
-	ulint		count		= 0;
 	char*		new_path;
 	char*		old_name;
 	char*		old_path;
@@ -2906,25 +2864,10 @@ fil_rename_tablespace(
 
 	ut_a(id != 0);
 
-retry:
-	count++;
-
-	if (!(count % 1000)) {
-		ut_print_timestamp(stderr);
-		fputs("  InnoDB: Warning: problems renaming ", stderr);
-		ut_print_filename(stderr,
-				  old_name_in ? old_name_in : not_given);
-		fputs(" to ", stderr);
-		ut_print_filename(stderr, new_name);
-		fprintf(stderr, ", %lu iterations\n", (ulong) count);
-	}
-
 	mutex_enter(&fil_system->mutex);
 
 	space = fil_space_get_by_id(id);
 
-	DBUG_EXECUTE_IF("fil_rename_tablespace_failure_1", space = NULL; );
-
 	if (space == NULL) {
 		ib_logf(IB_LOG_LEVEL_ERROR,
 			"Cannot find space id %lu in the tablespace "
@@ -2936,54 +2879,11 @@ retry:
 		return(FALSE);
 	}
 
-	if (count > 25000) {
-		space->stop_ios = FALSE;
-		mutex_exit(&fil_system->mutex);
-
-		return(FALSE);
-	}
-
-	/* We temporarily close the .ibd file because we do not trust that
-	operating systems can rename an open file. For the closing we have to
-	wait until there are no pending i/o's or flushes on the file. */
-
-	space->stop_ios = TRUE;
-
 	/* The following code must change when InnoDB supports
 	multiple datafiles per tablespace. */
 	ut_a(UT_LIST_GET_LEN(space->chain) == 1);
 	node = UT_LIST_GET_FIRST(space->chain);
 
-	if (node->n_pending > 0
-	    || node->n_pending_flushes > 0
-	    || node->being_extended) {
-		/* There are pending i/o's or flushes or the file is
-		currently being extended, sleep for a while and
-		retry */
-
-		mutex_exit(&fil_system->mutex);
-
-		os_thread_sleep(20000);
-
-		goto retry;
-
-	} else if (node->modification_counter > node->flush_counter) {
-		/* Flush the space */
-
-		mutex_exit(&fil_system->mutex);
-
-		os_thread_sleep(20000);
-
-		fil_flush(id);
-
-		goto retry;
-
-	} else if (node->open) {
-		/* Close the file */
-
-		fil_node_close_file(node, fil_system);
-	}
-
 	/* Check that the old name in the space is right */
 
 	if (old_name_in) {
@@ -3002,17 +2902,9 @@ retry:
 		space, node, new_name, new_path);
 
 	if (success) {
-
-		DBUG_EXECUTE_IF("fil_rename_tablespace_failure_2",
-			goto skip_second_rename; );
-
 		success = os_file_rename(
 			innodb_file_data_key, old_path, new_path);
 
-		DBUG_EXECUTE_IF("fil_rename_tablespace_failure_2",
-skip_second_rename:
-			success = FALSE; );
-
 		if (!success) {
 			/* We have to revert the changes we made
 			to the tablespace memory cache */
@@ -3022,8 +2914,6 @@ skip_second_rename:
 		}
 	}
 
-	space->stop_ios = FALSE;
-
 	mutex_exit(&fil_system->mutex);
 
 #ifndef UNIV_HOTBACKUP
diff --git a/storage/innobase/include/fil0fil.h b/storage/innobase/include/fil0fil.h
index 9374bb88bb7..6e772e31772 100644
--- a/storage/innobase/include/fil0fil.h
+++ b/storage/innobase/include/fil0fil.h
@@ -256,10 +256,6 @@ struct fil_space_t {
 				the space corresponds to a table in the InnoDB
 				data dictionary; so we can print a warning of
 				orphaned tablespaces */
-	ibool		stop_ios;/*!< TRUE if we want to rename the
-				.ibd file of tablespace and want to
-				stop temporarily posting of new i/o
-				requests on the file */
 	ibool		stop_new_ops;
 				/*!< we set this TRUE when we start
 				deleting a single-table tablespace.
diff --git a/storage/innobase/os/os0file.cc b/storage/innobase/os/os0file.cc
index 55d2c15f906..8f3f3716fc2 100644
--- a/storage/innobase/os/os0file.cc
+++ b/storage/innobase/os/os0file.cc
@@ -2,7 +2,7 @@
 
 Copyright (c) 1995, 2017, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2009, Percona Inc.
-Copyright (c) 2013, 2017, MariaDB Corporation.
+Copyright (c) 2013, 2018, MariaDB Corporation.
 
 Portions of this file contain modifications contributed and copyrighted
 by Percona Inc.. Those modifications are
@@ -1186,7 +1186,8 @@ os_file_create_simple_func(
 		/* Use default security attributes and no template file. */
 
 		file = CreateFile(
-			(LPCTSTR) name, access, FILE_SHARE_READ, NULL,
+			(LPCTSTR) name, access,
+			FILE_SHARE_READ | FILE_SHARE_DELETE, NULL,
 			create_flag, attributes, NULL);
 
 		if (file == INVALID_HANDLE_VALUE) {
@@ -1314,7 +1315,7 @@ os_file_create_simple_no_error_handling_func(
 	DWORD		access;
 	DWORD		create_flag;
 	DWORD		attributes	= 0;
-	DWORD		share_mode	= FILE_SHARE_READ;
+	DWORD		share_mode	= FILE_SHARE_READ | FILE_SHARE_DELETE;
 	ut_a(name);
 
 	ut_a(!(create_mode & OS_FILE_ON_ERROR_SILENT));
@@ -1554,7 +1555,7 @@ os_file_create_func(
 
 #ifdef __WIN__
 	DWORD		create_flag;
-	DWORD		share_mode	= FILE_SHARE_READ;
+	DWORD		share_mode	= FILE_SHARE_READ | FILE_SHARE_DELETE;
 
 	on_error_no_exit = create_mode & OS_FILE_ON_ERROR_NO_EXIT
 		? TRUE : FALSE;
diff --git a/storage/xtradb/fil/fil0fil.cc b/storage/xtradb/fil/fil0fil.cc
index 3807e6b3395..49c5f4b090b 100644
--- a/storage/xtradb/fil/fil0fil.cc
+++ b/storage/xtradb/fil/fil0fil.cc
@@ -862,7 +862,6 @@ fil_mutex_enter_and_prepare_for_io(
 	ibool		success;
 	ibool		print_info	= FALSE;
 	ulint		count		= 0;
-	ulint		count2		= 0;
 
 retry:
 	mutex_enter(&fil_system->mutex);
@@ -879,46 +878,6 @@ retry:
 
 	space = fil_space_get_by_id(space_id);
 
-	if (space != NULL && space->stop_ios) {
-		/* We are going to do a rename file and want to stop new i/o's
-		for a while */
-
-		if (count2 > 20000) {
-			fputs("InnoDB: Warning: tablespace ", stderr);
-			ut_print_filename(stderr, space->name);
-			fprintf(stderr,
-				" has i/o ops stopped for a long time %lu\n",
-				(ulong) count2);
-		}
-
-		mutex_exit(&fil_system->mutex);
-
-#ifndef UNIV_HOTBACKUP
-
-		/* Wake the i/o-handler threads to make sure pending
-		i/o's are performed */
-		os_aio_simulated_wake_handler_threads();
-
-		/* The sleep here is just to give IO helper threads a
-		bit of time to do some work. It is not required that
-		all IO related to the tablespace being renamed must
-		be flushed here as we do fil_flush() in
-		fil_rename_tablespace() as well. */
-		os_thread_sleep(20000);
-
-#endif /* UNIV_HOTBACKUP */
-
-		/* Flush tablespaces so that we can close modified
-		files in the LRU list */
-		fil_flush_file_spaces(FIL_TABLESPACE);
-
-		os_thread_sleep(20000);
-
-		count2++;
-
-		goto retry;
-	}
-
 	if (fil_system->n_open < fil_system->max_n_open) {
 
 		return;
@@ -2950,7 +2909,6 @@ fil_rename_tablespace(
 	ibool		success;
 	fil_space_t*	space;
 	fil_node_t*	node;
-	ulint		count		= 0;
 	char*		new_path;
 	char*		old_name;
 	char*		old_path;
@@ -2958,25 +2916,10 @@ fil_rename_tablespace(
 
 	ut_a(id != 0);
 
-retry:
-	count++;
-
-	if (!(count % 1000)) {
-		ut_print_timestamp(stderr);
-		fputs("  InnoDB: Warning: problems renaming ", stderr);
-		ut_print_filename(stderr,
-				  old_name_in ? old_name_in : not_given);
-		fputs(" to ", stderr);
-		ut_print_filename(stderr, new_name);
-		fprintf(stderr, ", %lu iterations\n", (ulong) count);
-	}
-
 	mutex_enter(&fil_system->mutex);
 
 	space = fil_space_get_by_id(id);
 
-	DBUG_EXECUTE_IF("fil_rename_tablespace_failure_1", space = NULL; );
-
 	if (space == NULL) {
 		ib_logf(IB_LOG_LEVEL_ERROR,
 			"Cannot find space id %lu in the tablespace "
@@ -2988,54 +2931,11 @@ retry:
 		return(FALSE);
 	}
 
-	if (count > 25000) {
-		space->stop_ios = FALSE;
-		mutex_exit(&fil_system->mutex);
-
-		return(FALSE);
-	}
-
-	/* We temporarily close the .ibd file because we do not trust that
-	operating systems can rename an open file. For the closing we have to
-	wait until there are no pending i/o's or flushes on the file. */
-
-	space->stop_ios = TRUE;
-
 	/* The following code must change when InnoDB supports
 	multiple datafiles per tablespace. */
 	ut_a(UT_LIST_GET_LEN(space->chain) == 1);
 	node = UT_LIST_GET_FIRST(space->chain);
 
-	if (node->n_pending > 0
-	    || node->n_pending_flushes > 0
-	    || node->being_extended) {
-		/* There are pending i/o's or flushes or the file is
-		currently being extended, sleep for a while and
-		retry */
-
-		mutex_exit(&fil_system->mutex);
-
-		os_thread_sleep(20000);
-
-		goto retry;
-
-	} else if (node->modification_counter > node->flush_counter) {
-		/* Flush the space */
-
-		mutex_exit(&fil_system->mutex);
-
-		os_thread_sleep(20000);
-
-		fil_flush(id);
-
-		goto retry;
-
-	} else if (node->open) {
-		/* Close the file */
-
-		fil_node_close_file(node, fil_system);
-	}
-
 	/* Check that the old name in the space is right */
 
 	if (old_name_in) {
@@ -3054,17 +2954,9 @@ retry:
 		space, node, new_name, new_path);
 
 	if (success) {
-
-		DBUG_EXECUTE_IF("fil_rename_tablespace_failure_2",
-			goto skip_second_rename; );
-
 		success = os_file_rename(
 			innodb_file_data_key, old_path, new_path);
 
-		DBUG_EXECUTE_IF("fil_rename_tablespace_failure_2",
-skip_second_rename:
-			success = FALSE; );
-
 		if (!success) {
 			/* We have to revert the changes we made
 			to the tablespace memory cache */
@@ -3074,8 +2966,6 @@ skip_second_rename:
 		}
 	}
 
-	space->stop_ios = FALSE;
-
 	mutex_exit(&fil_system->mutex);
 
 #ifndef UNIV_HOTBACKUP
diff --git a/storage/xtradb/include/fil0fil.h b/storage/xtradb/include/fil0fil.h
index 6230bd7a4d4..86b1c561349 100644
--- a/storage/xtradb/include/fil0fil.h
+++ b/storage/xtradb/include/fil0fil.h
@@ -249,10 +249,6 @@ struct fil_space_t {
 				the space corresponds to a table in the InnoDB
 				data dictionary; so we can print a warning of
 				orphaned tablespaces */
-	ibool		stop_ios;/*!< TRUE if we want to rename the
-				.ibd file of tablespace and want to
-				stop temporarily posting of new i/o
-				requests on the file */
 	ibool		stop_new_ops;
 				/*!< we set this TRUE when we start
 				deleting a single-table tablespace.
diff --git a/storage/xtradb/os/os0file.cc b/storage/xtradb/os/os0file.cc
index 5ed7cbbc9b3..c1ddc83d852 100644
--- a/storage/xtradb/os/os0file.cc
+++ b/storage/xtradb/os/os0file.cc
@@ -2,7 +2,7 @@
 
 Copyright (c) 1995, 2017, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2009, Percona Inc.
-Copyright (c) 2013, 2017, MariaDB Corporation.
+Copyright (c) 2013, 2018, MariaDB Corporation.
 
 Portions of this file contain modifications contributed and copyrighted
 by Percona Inc.. Those modifications are
@@ -1301,7 +1301,8 @@ os_file_create_simple_func(
 		/* Use default security attributes and no template file. */
 
 		file = CreateFile(
-			(LPCTSTR) name, access, FILE_SHARE_READ, NULL,
+			(LPCTSTR) name, access,
+			FILE_SHARE_READ | FILE_SHARE_DELETE, NULL,
 			create_flag, attributes, NULL);
 
 		if (file == INVALID_HANDLE_VALUE) {
@@ -1460,7 +1461,7 @@ os_file_create_simple_no_error_handling_func(
 	DWORD		access;
 	DWORD		create_flag;
 	DWORD		attributes	= 0;
-	DWORD		share_mode	= FILE_SHARE_READ;
+	DWORD		share_mode	= FILE_SHARE_READ | FILE_SHARE_DELETE;
 	ut_a(name);
 
 	ut_a(!(create_mode & OS_FILE_ON_ERROR_SILENT));
@@ -1737,7 +1738,7 @@ os_file_create_func(
 
 #ifdef __WIN__
 	DWORD		create_flag;
-	DWORD		share_mode	= FILE_SHARE_READ;
+	DWORD		share_mode	= FILE_SHARE_READ | FILE_SHARE_DELETE;
 
 	on_error_no_exit = create_mode & OS_FILE_ON_ERROR_NO_EXIT
 		? TRUE : FALSE;