MDEV-12091 Shutdown fails to wait for rollback of recovered transacti…

…ons to finish In the 10.1 InnoDB Plugin, a call os_event_free(buf_flush_event) was misplaced. The event could be signalled by rollback of resurrected transactions while shutdown was in progress. This bug was caught by cmake -DWITH_ASAN testing. This call was only present in the 10.1 InnoDB Plugin, not in other versions, or in XtraDB. That said, the bug affects all InnoDB versions. Shutdown assumes the cessation of any page-dirtying activity, including the activity of the background rollback thread. InnoDB only waited for the background rollback to finish as part of a slow shutdown (innodb_fast_shutdown=0). The default is a clean shutdown (innodb_fast_shutdown=1). In a scenario where InnoDB is killed, restarted, and shut down soon enough, the data files could become corrupted. logs_empty_and_mark_files_at_shutdown(): Wait for the rollback to finish, except if innodb_fast_shutdown=2 (crash-like shutdown) was requested. trx_rollback_or_clean_recovered(): Before choosing the next recovered transaction to roll back, terminate early if non-slow shutdown was initiated. Roll back everything on slow shutdown (innodb_fast_shutdown=0). srv_innodb_monitor_mutex: Declare as static, because the mutex is only used within one module. After each call to os_event_free(), ensure that the freed event is not reachable via global variables, by setting the relevant variables to NULL.
MariaDB · Mar 10, 2017 · 032678a · 032678a
1 parent 1d47bd6
commit 032678a
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 44 deletions.
diff --git a/storage/innobase/log/log0log.cc b/storage/innobase/log/log0log.cc
@@ -3207,12 +3207,6 @@ logs_empty_and_mark_files_at_shutdown(void)
 
 	ib_logf(IB_LOG_LEVEL_INFO, "Starting shutdown...");
 
-	while (srv_fast_shutdown == 0 && trx_rollback_or_clean_is_active) {
-		/* we should wait until rollback after recovery end
-		for slow shutdown */
-		os_thread_sleep(100000);
-	}
-
 	/* Wait until the master thread and all other operations are idle: our
 	algorithm only works if the server is idle at shutdown */
 
@@ -3264,7 +3258,8 @@ logs_empty_and_mark_files_at_shutdown(void)
 
 	active_thd = srv_get_active_thread_type();
 
-	if (active_thd != SRV_NONE) {
+	if (active_thd != SRV_NONE
+	    || (srv_fast_shutdown != 2 && trx_rollback_or_clean_is_active)) {
 
 		if (active_thd == SRV_PURGE) {
 			srv_purge_wakeup();
@@ -3280,11 +3275,9 @@ logs_empty_and_mark_files_at_shutdown(void)
 
 			switch (active_thd) {
 			case SRV_NONE:
-				/* This shouldn't happen because we've
-				already checked for this case before
-				entering the if(). We handle it here
-				to avoid a compiler warning. */
-				ut_error;
+				thread_type = "rollback of"
+					" recovered transactions";
+				break;
 			case SRV_WORKER:
 				thread_type = "worker threads";
 				break;
@@ -3709,7 +3702,6 @@ log_shutdown(void)
 
 #ifdef UNIV_LOG_ARCHIVE
 	rw_lock_free(&log_sys->archive_lock);
-	os_event_create();
 #endif /* UNIV_LOG_ARCHIVE */
 
 #ifdef UNIV_LOG_DEBUG

diff --git a/storage/innobase/srv/srv0srv.cc b/storage/innobase/srv/srv0srv.cc
@@ -3,7 +3,7 @@
 Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2008, 2009 Google Inc.
 Copyright (c) 2009, Percona Inc.
-Copyright (c) 2013, 2017, MariaDB Corporation Ab. All Rights Reserved.
+Copyright (c) 2013, 2017, MariaDB Corporation.
 
 Portions of this file contain modifications contributed and copyrighted by
 Google, Inc. Those modifications are gratefully acknowledged and are described
@@ -411,7 +411,7 @@ UNIV_INTERN const char* srv_io_thread_function[SRV_MAX_N_IO_THREADS];
 
 UNIV_INTERN time_t	srv_last_monitor_time;
 
-UNIV_INTERN ib_mutex_t	srv_innodb_monitor_mutex;
+static ib_mutex_t	srv_innodb_monitor_mutex;
 
 /* Mutex for locking srv_monitor_file. Not created if srv_read_only_mode */
 UNIV_INTERN ib_mutex_t	srv_monitor_file_mutex;

diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc
@@ -3086,7 +3086,7 @@ innobase_shutdown_for_mysql(void)
 	srv_free();
 	fil_close();
 
-	/* 4. Free the os_conc_mutex and all os_events and os_mutexes */
+	/* 4. Free all os_events and os_mutexes */
 
 	os_sync_free();
 

diff --git a/storage/innobase/trx/trx0roll.cc b/storage/innobase/trx/trx0roll.cc
@@ -1,6 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 2016, 2017, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -739,9 +740,9 @@ trx_rollback_or_clean_recovered(
 	}
 
 	if (all) {
-		fprintf(stderr,
-			"InnoDB: Starting in background the rollback"
-			" of uncommitted transactions\n");
+		ib_logf(IB_LOG_LEVEL_INFO,
+			"Starting in background the rollback"
+			" of recovered transactions");
 	}
 
 	/* Note: For XA recovered transactions, we rely on MySQL to
@@ -761,6 +762,12 @@ trx_rollback_or_clean_recovered(
 
 			assert_trx_in_rw_list(trx);
 
+			if (srv_shutdown_state != SRV_SHUTDOWN_NONE
+			    && srv_fast_shutdown != 0) {
+				all = FALSE;
+				break;
+			}
+
 			/* If this function does a cleanup or rollback
 			then it will release the trx_sys->mutex, therefore
 			we need to reacquire it before retrying the loop. */
@@ -778,10 +785,8 @@ trx_rollback_or_clean_recovered(
 	} while (trx != NULL);
 
 	if (all) {
-		ut_print_timestamp(stderr);
-		fprintf(stderr,
-			"  InnoDB: Rollback of non-prepared"
-			" transactions completed\n");
+		ib_logf(IB_LOG_LEVEL_INFO,
+			"Rollback of non-prepared transactions completed");
 	}
 }
 

diff --git a/storage/xtradb/log/log0log.cc b/storage/xtradb/log/log0log.cc
@@ -3512,12 +3512,6 @@ logs_empty_and_mark_files_at_shutdown(void)
 	if (log_disable_checkpoint_active)
 		log_enable_checkpoint();
 
-	while (srv_fast_shutdown == 0 && trx_rollback_or_clean_is_active) {
-		/* we should wait until rollback after recovery end
-		for slow shutdown */
-		os_thread_sleep(100000);
-	}
-
 	/* Wait until the master thread and all other operations are idle: our
 	algorithm only works if the server is idle at shutdown */
 
@@ -3569,7 +3563,8 @@ logs_empty_and_mark_files_at_shutdown(void)
 
 	active_thd = srv_get_active_thread_type();
 
-	if (active_thd != SRV_NONE) {
+	if (active_thd != SRV_NONE
+	    || (srv_fast_shutdown != 2 && trx_rollback_or_clean_is_active)) {
 
 		if (active_thd == SRV_PURGE) {
 			srv_purge_wakeup();
@@ -3585,11 +3580,9 @@ logs_empty_and_mark_files_at_shutdown(void)
 
 			switch (active_thd) {
 			case SRV_NONE:
-				/* This shouldn't happen because we've
-				already checked for this case before
-				entering the if(). We handle it here
-				to avoid a compiler warning. */
-				ut_error;
+				thread_type = "rollback of"
+					" recovered transactions";
+				break;
 			case SRV_WORKER:
 				thread_type = "worker threads";
 				break;

diff --git a/storage/xtradb/srv/srv0srv.cc b/storage/xtradb/srv/srv0srv.cc
@@ -3,7 +3,7 @@
 Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2008, 2009 Google Inc.
 Copyright (c) 2009, Percona Inc.
-Copyright (c) 2013, 2017, MariaDB Corporation Ab. All Rights Reserved.
+Copyright (c) 2013, 2017, MariaDB Corporation.
 
 Portions of this file contain modifications contributed and copyrighted by
 Google, Inc. Those modifications are gratefully acknowledged and are described
@@ -543,7 +543,7 @@ UNIV_INTERN const char* srv_io_thread_function[SRV_MAX_N_IO_THREADS];
 
 UNIV_INTERN time_t	srv_last_monitor_time;
 
-UNIV_INTERN ib_mutex_t	srv_innodb_monitor_mutex;
+static ib_mutex_t	srv_innodb_monitor_mutex;
 
 /* Mutex for locking srv_monitor_file. Not created if srv_read_only_mode */
 UNIV_INTERN ib_mutex_t	srv_monitor_file_mutex;
@@ -1209,10 +1209,15 @@ srv_free(void)
 			os_event_free(srv_sys->sys_threads[i].event);
 
 		os_event_free(srv_error_event);
+		srv_error_event = NULL;
 		os_event_free(srv_monitor_event);
+		srv_monitor_event = NULL;
 		os_event_free(srv_buf_dump_event);
+		srv_buf_dump_event = NULL;
 		os_event_free(srv_checkpoint_completed_event);
+		srv_checkpoint_completed_event = NULL;
 		os_event_free(srv_redo_log_tracked_event);
+		srv_redo_log_tracked_event = NULL;
 		mutex_free(&srv_sys->mutex);
 		mutex_free(&srv_sys->tasks_mutex);
 	}

diff --git a/storage/xtradb/trx/trx0roll.cc b/storage/xtradb/trx/trx0roll.cc
@@ -1,6 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 2016, 2017, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -739,9 +740,9 @@ trx_rollback_or_clean_recovered(
 	}
 
 	if (all) {
-		fprintf(stderr,
-			"InnoDB: Starting in background the rollback"
-			" of uncommitted transactions\n");
+		ib_logf(IB_LOG_LEVEL_INFO,
+			"Starting in background the rollback"
+			" of recovered transactions");
 	}
 
 	/* Note: For XA recovered transactions, we rely on MySQL to
@@ -761,6 +762,12 @@ trx_rollback_or_clean_recovered(
 
 			assert_trx_in_rw_list(trx);
 
+			if (srv_shutdown_state != SRV_SHUTDOWN_NONE
+			    && srv_fast_shutdown != 0) {
+				all = FALSE;
+				break;
+			}
+
 			/* If this function does a cleanup or rollback
 			then it will release the trx_sys->mutex, therefore
 			we need to reacquire it before retrying the loop. */
@@ -778,10 +785,8 @@ trx_rollback_or_clean_recovered(
 	} while (trx != NULL);
 
 	if (all) {
-		ut_print_timestamp(stderr);
-		fprintf(stderr,
-			"  InnoDB: Rollback of non-prepared"
-			" transactions completed\n");
+		ib_logf(IB_LOG_LEVEL_INFO,
+			"Rollback of non-prepared transactions completed");
 	}
 }