List:Commits« Previous MessageNext Message »
From:vasil.dimov Date:June 16 2010 12:26pm
Subject:bzr commit into mysql-next-mr-persistent-stats branch (vasil.dimov:3231)
View as plain text  
#At file:///usr/local/devel/bzrroot/server/mysql-next-mr-persistent-stats/ based on revid:vasil.dimov@stripped

 3231 Vasil Dimov	2010-06-16
      Resolve possible waits in DROP TABLE
      
      * Add a new flag allowed_to_wait to trx_t that when FALSE has the same
      effect as thd_lock_wait_timeout() being 0 for this trx.
      
      * Set allowed_to_wait to FALSE for the private trx that is doing
      DELETE FROM table_stats WHERE table_name='t'; inside DROP TABLE t;
      If the DBA has locked the rows in table_stats then DROP will return
      an error.
      
      * Ensure that the stats tables will not get DROPped when
      dict_stats_drop_table() is executing by inrementing the table reference
      count. Currently MySQL is protecting us with the LOCK_open mutex, but
      there are plans to remove it.
      
      * Remove now unnecessary function lock_table_by_name()

    modified:
      storage/innobase/dict/dict0stats.c
      storage/innobase/handler/ha_innodb.cc
      storage/innobase/include/dict0stats.h
      storage/innobase/include/trx0trx.h
      storage/innobase/lock/lock0lock.c
      storage/innobase/row/row0mysql.c
      storage/innobase/srv/srv0srv.c
      storage/innobase/trx/trx0trx.c
=== modified file 'storage/innobase/dict/dict0stats.c'
--- a/storage/innobase/dict/dict0stats.c	revid:vasil.dimov@stripped
+++ b/storage/innobase/dict/dict0stats.c	revid:vasil.dimov@stripped
@@ -167,8 +167,8 @@ dict_stats_table_check(
 
 	table = dict_table_get_low(req_schema->table_name);
 
-	if (table == NULL) {
-		/* no such table */
+	if (table == NULL || table->ibd_file_missing) {
+		/* no such table or missing tablespace */
 
 		return(FALSE);
 	}
@@ -366,14 +366,23 @@ dict_stats_persistent_storage_check()
 		index_stats_columns
 	};
 
+	ibool	caller_has_dict_sys_mutex;
 	ibool	ret;
 
-	mutex_enter(&(dict_sys->mutex));
+	caller_has_dict_sys_mutex = mutex_own(&dict_sys->mutex);
+
+	if (!caller_has_dict_sys_mutex) {
+		mutex_enter(&(dict_sys->mutex));
+	}
+
+	ut_ad(mutex_own(&dict_sys->mutex));
 
 	ret = dict_stats_table_check(&table_stats_schema)
 	   && dict_stats_table_check(&index_stats_schema);
 
-	mutex_exit(&(dict_sys->mutex));
+	if (!caller_has_dict_sys_mutex) {
+		mutex_exit(&(dict_sys->mutex));
+	}
 
 	return(ret);
 }
@@ -2336,22 +2345,32 @@ dict_stats_drop_index(
 Removes the statistics for a table and all of its indexes from the
 persistent storage if it exists and if there is data stored for the table.
 This function creates its own transaction and commits it.
-dict_stats_drop_table() @{ */
+dict_stats_drop_table() @{
+@return DB_SUCCESS or error code */
 UNIV_INTERN
-void
+enum db_err
 dict_stats_drop_table(
 /*==================*/
 	const char*	table_name)	/*!< in: table name */
 {
 	trx_t*		trx;
 	pars_info_t*	pinfo;
-	ulint		ret;
+	enum db_err	ret = DB_ERROR;
+	dict_table_t*	table_stats;
+	dict_table_t*	index_stats;
 
 	/* skip tables that do not contain a database name
 	e.g. if we are dropping SYS_TABLES */
 	if (strchr(table_name, '/') == NULL) {
 
-		return;
+		return(DB_SUCCESS);
+	}
+
+	/* skip table_stats and index_stats themselves */
+	if (strcmp(table_name, TABLE_STATS_NAME) == 0
+	    || strcmp(table_name, INDEX_STATS_NAME) == 0) {
+
+		return(DB_SUCCESS);
 	}
 
 	mutex_enter(&kernel_mutex);
@@ -2360,32 +2379,39 @@ dict_stats_drop_table(
 
 	trx->op_info = "";
 	trx->isolation_level = TRX_ISO_READ_UNCOMMITTED;
+	trx->allowed_to_wait = FALSE;
 	trx_start(trx, ULINT_UNDEFINED);
 
-	/* Cannot continue without locking the stats tables
-	because the SQL parser will crash if they disappear after
-	dict_stats_persistent_storage_check(). The tables will be
-	unlocked when the transaction is committed. */
-
-	/* XXX those locks still do not prevent the table from being dropped */
-
-	if (lock_table_by_name(TABLE_STATS_NAME, LOCK_X, trx) != DB_SUCCESS) {
-
+	/* Increment table reference count to prevent the tables from
+	being DROPped just before que_eval_sql(). As of the time of
+	this code is written MySQL's LOCK_open mutex protects us here
+	from races, but we nevertheless increment the ref counter to
+	be sure the code continues to be race-free even if MySQL code
+	is changed. */
+
+	row_mysql_lock_data_dictionary(trx);
+
+	table_stats = dict_table_get_low(TABLE_STATS_NAME);
+	index_stats = dict_table_get_low(INDEX_STATS_NAME);
+
+	if (table_stats == NULL || index_stats == NULL) {
+		/* tables do not exist */
+		row_mysql_unlock_data_dictionary(trx);
+		ret = DB_SUCCESS;
 		goto commit_and_return;
 	}
 
-	if (lock_table_by_name(INDEX_STATS_NAME, LOCK_X, trx) != DB_SUCCESS) {
+	table_stats->n_mysql_handles_opened++;
+	index_stats->n_mysql_handles_opened++;
 
-		/* the commit will unlock table_stats */
-		goto commit_and_return;
-	}
+	row_mysql_unlock_data_dictionary(trx);
 
 	/* If the persistent storage does not exist or is corrupted,
 	then do not attempt to DELETE from its tables because the
 	internal SQL parser will crash. */
 	if (!dict_stats_persistent_storage_check()) {
 
-		/* the commit will unlock the stats tables */
+		ret = DB_SUCCESS;
 		goto commit_and_return;
 	}
 
@@ -2421,15 +2447,33 @@ dict_stats_drop_table(
 
 	/* pinfo is freed by que_eval_sql() */
 
-	ut_a(ret == DB_SUCCESS);
+	ut_a(ret == DB_SUCCESS || ret == DB_LOCK_WAIT_TIMEOUT);
+
+	if (ret == DB_LOCK_WAIT_TIMEOUT) {
+
+		ut_print_timestamp(stderr);
+		fprintf(stderr,
+			" InnoDB: Unable to instantly delete statistics "
+			"for %s from %s or %s.\n",
+			table_name, TABLE_STATS_NAME, INDEX_STATS_NAME);
+		ut_print_timestamp(stderr);
+		fprintf(stderr,
+			" InnoDB: Please unlock the rows in those tables and "
+			"retry the operation.\n");
+	}
 
 commit_and_return:
 
+	dict_table_decrement_handle_count(table_stats, FALSE);
+	dict_table_decrement_handle_count(index_stats, FALSE);
+
 	trx_commit_for_mysql(trx);
 
 	mutex_enter(&kernel_mutex);
 	trx_free(trx);
 	mutex_exit(&kernel_mutex);
+
+	return(ret);
 }
 /* @} */
 

=== modified file 'storage/innobase/handler/ha_innodb.cc'
--- a/storage/innobase/handler/ha_innodb.cc	revid:vasil.dimov@stripped
+++ b/storage/innobase/handler/ha_innodb.cc	revid:vasil.dimov@stripped
@@ -7130,6 +7130,16 @@ ha_innobase::delete_table(
 		DBUG_RETURN(HA_ERR_GENERIC);
 	}
 
+	/* Remove stats for this table and all of its indexes from the
+	persistent storage if it exists and if there are stats for this
+	table in there. This function creates its own trx and commits
+	it. */
+	error = dict_stats_drop_table(norm_name);
+	if (error != DB_SUCCESS) {
+		error = convert_error_code_to_mysql(error, 0, NULL);
+		DBUG_RETURN(error);
+	}
+
 	/* Get the transaction associated with the current thd, or create one
 	if not yet created */
 

=== modified file 'storage/innobase/include/dict0stats.h'
--- a/storage/innobase/include/dict0stats.h	revid:vasil.dimov@stripped
+++ b/storage/innobase/include/dict0stats.h	revid:vasil.dimov@stripped
@@ -81,9 +81,10 @@ dict_stats_drop_index(
 /*********************************************************************//**
 Removes the statistics for a table and all of its indexes from the
 persistent storage if it exists and if there is data stored for the table.
-This function creates its own transaction and commits it. */
+This function creates its own transaction and commits it.
+@return DB_SUCCESS or error code */
 UNIV_INTERN
-void
+enum db_err
 dict_stats_drop_table(
 /*==================*/
 	const char*	table);	/*!< in: table */

=== modified file 'storage/innobase/include/trx0trx.h'
--- a/storage/innobase/include/trx0trx.h	revid:vasil.dimov@stripped
+++ b/storage/innobase/include/trx0trx.h	revid:vasil.dimov@stripped
@@ -644,6 +644,11 @@ struct trx_struct{
 					TRX_QUE_LOCK_WAIT, this points to
 					the lock request, otherwise this is
 					NULL */
+	ibool		allowed_to_wait;/*!< if this is set to FALSE then
+					we consider lock_wait_timeout==0 for
+					this trx instead of looking at
+					thd_lock_wait_timeout(trx->mysql_thd)
+					*/
 	ibool		was_chosen_as_deadlock_victim;
 					/* when the transaction decides to wait
 					for a lock, it sets this to FALSE;

=== modified file 'storage/innobase/lock/lock0lock.c'
--- a/storage/innobase/lock/lock0lock.c	revid:vasil.dimov@stripped
+++ b/storage/innobase/lock/lock0lock.c	revid:vasil.dimov@stripped
@@ -3883,91 +3883,6 @@ lock_table(
 }
 
 /*********************************************************************//**
-Lock a given table with the given lock mode. This function may block the
-execution for some time, but will not wait infinitely if the table is
-already locked, it may return DB_LOCK_WAIT_TIMEOUT.
-The table is unlocked when the trx is committed.
-@return DB_SUCCESS or error code */
-UNIV_INTERN
-enum db_err
-lock_table_by_name(
-/*===============*/
-	const char*	table_name,	/*!< in: table name */
-	enum lock_mode	mode,		/*!< in: lock mode */
-	trx_t*		trx)		/*!< in/out: transaction into which to
-					lock the table */
-{
-	mem_heap_t*	heap;
-	sel_node_t*	node;
-	que_fork_t*	graph;
-	que_thr_t*	thr;
-	dict_table_t*	table;
-	ibool		was_lock_wait;
-	ulint		ret;
-
-	trx->op_info = "locking table";
-
-	heap = mem_heap_create(sizeof(sel_node_t)
-			       + sizeof(que_fork_t)
-			       + sizeof(que_thr_t));
-
-	node = sel_node_create(heap);
-
-	graph = que_node_get_parent(
-			pars_complete_graph_for_exec(node, trx, heap));
-
-	graph->state = QUE_FORK_ACTIVE;
-
-	thr = que_fork_get_first_thr(graph);
-
-	que_thr_move_to_run_state_for_mysql(thr, trx);
-
-	do {
-		mutex_enter(&dict_sys->mutex);
-
-		table = dict_table_get_low(table_name);
-
-		if (table == NULL) {
-
-			mutex_exit(&dict_sys->mutex);
-
-			ret = DB_TABLE_NOT_FOUND;
-
-			break;
-		}
-
-		thr->run_node = thr;
-		thr->prev_node = thr->common.parent;
-
-		/* try lock */
-		ret = lock_table(0, table, mode, thr);
-
-		mutex_exit(&dict_sys->mutex);
-
-		trx->error_state = ret;
-
-		if (ret == DB_SUCCESS) {
-
-			que_thr_stop_for_mysql_no_error(thr, trx);
-
-			was_lock_wait = FALSE;
-		} else {
-
-			que_thr_stop_for_mysql(thr);
-
-			was_lock_wait = row_mysql_handle_errors(&ret, trx,
-								thr, NULL);
-		}
-	} while (was_lock_wait);
-
-	mem_heap_free(heap);
-
-	trx->op_info = "";
-
-	return(ret);
-}
-
-/*********************************************************************//**
 Checks if a waiting table lock request still has to wait in a queue.
 @return	TRUE if still has to wait */
 static

=== modified file 'storage/innobase/row/row0mysql.c'
--- a/storage/innobase/row/row0mysql.c	revid:vasil.dimov@stripped
+++ b/storage/innobase/row/row0mysql.c	revid:vasil.dimov@stripped
@@ -3007,12 +3007,6 @@ row_drop_table_for_mysql(
 		srv_print_innodb_table_monitor = FALSE;
 	}
 
-	/* Remove stats for this table and all of its indexes from the
-	persistent storage if it exists and if there are stats for this
-	table in there. This function creates its own trx and commits
-	it. */
-	dict_stats_drop_table(name);
-
 	/* Serialize data dictionary operations with dictionary mutex:
 	no deadlocks can occur then in these operations */
 

=== modified file 'storage/innobase/srv/srv0srv.c'
--- a/storage/innobase/srv/srv0srv.c	revid:vasil.dimov@stripped
+++ b/storage/innobase/srv/srv0srv.c	revid:vasil.dimov@stripped
@@ -496,6 +496,11 @@ intervals. Following macros define thres
 #define SRV_RECENT_IO_ACTIVITY	(PCT_IO(5))
 #define SRV_PAST_IO_ACTIVITY	(PCT_IO(200))
 
+#define fetch_lock_wait_timeout(trx)			\
+	((trx)->allowed_to_wait				\
+	 ? thd_lock_wait_timeout((trx)->mysql_thd)	\
+	 : 0)
+
 /*
 	IMPLEMENTATION OF THE SERVER MAIN PROGRAM
 	=========================================
@@ -1650,7 +1655,7 @@ srv_suspend_mysql_thread(
 	incomplete transactions that are being rolled back after crash
 	recovery) will use the global value of
 	innodb_lock_wait_timeout, because trx->mysql_thd == NULL. */
-	lock_wait_timeout = thd_lock_wait_timeout(trx->mysql_thd);
+	lock_wait_timeout = fetch_lock_wait_timeout(trx);
 
 	if (lock_wait_timeout < 100000000
 	    && wait_time > (double) lock_wait_timeout) {
@@ -2216,8 +2221,7 @@ loop:
 			wait_time = ut_difftime(ut_time(), slot->suspend_time);
 
 			trx = thr_get_trx(slot->thr);
-			lock_wait_timeout = thd_lock_wait_timeout(
-				trx->mysql_thd);
+			lock_wait_timeout = fetch_lock_wait_timeout(trx);
 
 			if (trx_is_interrupted(trx)
 			    || (lock_wait_timeout < 100000000

=== modified file 'storage/innobase/trx/trx0trx.c'
--- a/storage/innobase/trx/trx0trx.c	revid:vasil.dimov@stripped
+++ b/storage/innobase/trx/trx0trx.c	revid:vasil.dimov@stripped
@@ -160,6 +160,7 @@ trx_create(
 	trx->graph = NULL;
 
 	trx->wait_lock = NULL;
+	trx->allowed_to_wait = TRUE;
 	trx->was_chosen_as_deadlock_victim = FALSE;
 	UT_LIST_INIT(trx->wait_thrs);
 


Attachment: [text/bzr-bundle] bzr/vasil.dimov@oracle.com-20100616122027-p8sfw0xbcq9hi1vr.bundle
Thread
bzr commit into mysql-next-mr-persistent-stats branch (vasil.dimov:3231) vasil.dimov16 Jun