Applying InnoDB snashot 5.1-ss5024,part 3. Fixes BUG#42101

BUG#42101 - Race condition in innodb_commit_concurrency

Detailed revision comments:

r4994 | marko | 2009-05-14 15:04:55 +0300 (Thu, 14 May 2009) | 18 lines
branches/5.1: Prevent a race condition in innobase_commit() by ensuring
that innodb_commit_concurrency>0 remains constant at run time. (Bug #42101)

srv_commit_concurrency: Make this a static variable in ha_innodb.cc.

innobase_commit_concurrency_validate(): Check that innodb_commit_concurrency
is not changed from or to 0 at run time.  This is needed, because
innobase_commit() assumes that innodb_commit_concurrency>0 remains constant.
Without this limitation, the checks for innodb_commit_concurrency>0
in innobase_commit() should be removed and that function would have to
acquire and release commit_cond_m at least twice per invocation.
Normally, innodb_commit_concurrency=0, and introducing the mutex operations
would mean significant overhead.

innodb_bug42101.test, innodb_bug42101-nonzero.test: Test cases.

rb://123 approved by Heikki Tuuri

mysql-test/r/innodb_bug42101-nonzero.result

+set global innodb_commit_concurrency=0;
+ERROR HY000: Incorrect arguments to SET
+select @@innodb_commit_concurrency;
+@@innodb_commit_concurrency
+1
+set global innodb_commit_concurrency=1;
+select @@innodb_commit_concurrency;
+@@innodb_commit_concurrency
+1
+set global innodb_commit_concurrency=42;
+select @@innodb_commit_concurrency;
+@@innodb_commit_concurrency
+42
+set global innodb_commit_concurrency=0;
+ERROR HY000: Incorrect arguments to SET
+select @@innodb_commit_concurrency;
+@@innodb_commit_concurrency
+42
+set global innodb_commit_concurrency=1;
+select @@innodb_commit_concurrency;
+@@innodb_commit_concurrency
+1

mysql-test/r/innodb_bug42101.result

+set global innodb_commit_concurrency=0;
+select @@innodb_commit_concurrency;
+@@innodb_commit_concurrency
+0
+set global innodb_commit_concurrency=1;
+ERROR HY000: Incorrect arguments to SET
+select @@innodb_commit_concurrency;
+@@innodb_commit_concurrency
+0
+set global innodb_commit_concurrency=42;
+ERROR HY000: Incorrect arguments to SET
+select @@innodb_commit_concurrency;
+@@innodb_commit_concurrency
+0
+set global innodb_commit_concurrency=0;
+select @@innodb_commit_concurrency;
+@@innodb_commit_concurrency
+0

mysql-test/t/innodb_bug42101-nonzero-master.opt

+--innodb_commit_concurrency=1

mysql-test/t/innodb_bug42101-nonzero.test

+#
+# Bug#42101  	Race condition in innodb_commit_concurrency
+# http://bugs.mysql.com/42101
+#
+
+-- source include/have_innodb.inc
+
+--error ER_WRONG_ARGUMENTS
+set global innodb_commit_concurrency=0;
+select @@innodb_commit_concurrency;
+set global innodb_commit_concurrency=1;
+select @@innodb_commit_concurrency;
+set global innodb_commit_concurrency=42;
+select @@innodb_commit_concurrency;
+--error ER_WRONG_ARGUMENTS
+set global innodb_commit_concurrency=0;
+select @@innodb_commit_concurrency;
+set global innodb_commit_concurrency=1;
+select @@innodb_commit_concurrency;

mysql-test/t/innodb_bug42101.test

+#
+# Bug#42101  	Race condition in innodb_commit_concurrency
+# http://bugs.mysql.com/42101
+#
+
+-- source include/have_innodb.inc
+
+set global innodb_commit_concurrency=0;
+select @@innodb_commit_concurrency;
+--error ER_WRONG_ARGUMENTS
+set global innodb_commit_concurrency=1;
+select @@innodb_commit_concurrency;
+--error ER_WRONG_ARGUMENTS
+set global innodb_commit_concurrency=42;
+select @@innodb_commit_concurrency;
+set global innodb_commit_concurrency=0;
+select @@innodb_commit_concurrency;

storage/innobase/handler/ha_innodb.cc

@@ -101,6 +101,7 @@ static long innobase_mirrored_log_groups, innobase_log_files_in_group,
 	innobase_additional_mem_pool_size, innobase_file_io_threads,
 	innobase_lock_wait_timeout, innobase_force_recovery,
 	innobase_open_files, innobase_autoinc_lock_mode;
+static ulong innobase_commit_concurrency = 0;
 
 static long long innobase_buffer_pool_size, innobase_log_file_size;
 
@@ -165,6 +166,38 @@ static handler *innobase_create_handler(handlerton *hton,
 
 static const char innobase_hton_name[]= "InnoDB";
 
+/*****************************************************************
+Check for a valid value of innobase_commit_concurrency. */
+static
+int
+innobase_commit_concurrency_validate(
+/*=================================*/
+						/* out: 0 for valid
+						innodb_commit_concurrency */
+	THD*				thd,	/* in: thread handle */
+	struct st_mysql_sys_var*	var,	/* in: pointer to system
+						variable */
+	void*				save,	/* out: immediate result
+						for update function */
+	struct st_mysql_value*		value)	/* in: incoming string */
+{
+	long long	intbuf;
+	ulong		commit_concurrency;
+
+	DBUG_ENTER("innobase_commit_concurrency_validate");
+
+	if (value->val_int(value, &intbuf)) {
+		/* The value is NULL. That is invalid. */
+		DBUG_RETURN(1);
+	}
+
+	*reinterpret_cast<ulong*>(save) = commit_concurrency
+		= static_cast<ulong>(intbuf);
+
+	/* Allow the value to be updated, as long as it remains zero
+	or nonzero. */
+	DBUG_RETURN(!(!commit_concurrency == !innobase_commit_concurrency));
+}
 
 static MYSQL_THDVAR_BOOL(support_xa, PLUGIN_VAR_OPCMDARG,
   "Enable InnoDB support for the XA two-phase commit",
@@ -1951,11 +1984,11 @@ innobase_commit(
 		Note, the position is current because of
 		prepare_commit_mutex */
 retry:
-		if (srv_commit_concurrency > 0) {
+		if (innobase_commit_concurrency > 0) {
 			pthread_mutex_lock(&commit_cond_m);
 			commit_threads++;
 
-			if (commit_threads > srv_commit_concurrency) {
+			if (commit_threads > innobase_commit_concurrency) {
 				commit_threads--;
 				pthread_cond_wait(&commit_cond,
 					&commit_cond_m);
@@ -1972,7 +2005,7 @@ innobase_commit(
 
 		innobase_commit_low(trx);
 
-		if (srv_commit_concurrency > 0) {
+		if (innobase_commit_concurrency > 0) {
 			pthread_mutex_lock(&commit_cond_m);
 			commit_threads--;
 			pthread_cond_signal(&commit_cond);
@@ -8289,10 +8322,10 @@ static MYSQL_SYSVAR_LONGLONG(buffer_pool_size, innobase_buffer_pool_size,
   "The size of the memory buffer InnoDB uses to cache data and indexes of its tables.",
   NULL, NULL, 8*1024*1024L, 1024*1024L, LONGLONG_MAX, 1024*1024L);
 
-static MYSQL_SYSVAR_ULONG(commit_concurrency, srv_commit_concurrency,
+static MYSQL_SYSVAR_ULONG(commit_concurrency, innobase_commit_concurrency,
   PLUGIN_VAR_RQCMDARG,
   "Helps in performance tuning in heavily concurrent environments.",
-  NULL, NULL, 0, 0, 1000, 0);
+  innobase_commit_concurrency_validate, NULL, 0, 0, 1000, 0);
 
 static MYSQL_SYSVAR_ULONG(concurrency_tickets, srv_n_free_tickets_to_enter,
   PLUGIN_VAR_RQCMDARG,

storage/innobase/include/srv0srv.h

@@ -109,7 +109,6 @@ extern ulint	srv_max_dirty_pages_pct;
 
 extern ulint	srv_force_recovery;
 extern ulong	srv_thread_concurrency;
-extern ulong	srv_commit_concurrency;
 
 extern ulint	srv_max_n_threads;
 

storage/innobase/srv/srv0srv.c

@@ -285,7 +285,6 @@ computer. Bigger computers need bigger values. Value 0 will disable the
 concurrency check. */
 
 ulong	srv_thread_concurrency	= 0;
-ulong	srv_commit_concurrency	= 0;
 
 os_fast_mutex_t	srv_conc_mutex;		/* this mutex protects srv_conc data
 					structures */