From: Satya B Date: May 19 2009 8:20am Subject: bzr commit into mysql-5.1-bugteam branch (satya.bn:2891) Bug#42101 List-Archive: http://lists.mysql.com/commits/74460 X-Bug: 42101 Message-Id: <0KJV00747TU61VJ0@mail-apac.sun.com> MIME-Version: 1.0 Content-Type: text/plain; CHARSET=US-ASCII Content-Transfer-Encoding: 7BIT #At file:///home/satya/WORK/mysql/mysql-5.1-bugteam-innodb/ based on revid:satya.bn@stripped 2891 Satya B 2009-05-19 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 added: mysql-test/r/innodb_bug42101-nonzero.result mysql-test/r/innodb_bug42101.result mysql-test/t/innodb_bug42101-nonzero-master.opt mysql-test/t/innodb_bug42101-nonzero.test mysql-test/t/innodb_bug42101.test modified: storage/innobase/handler/ha_innodb.cc storage/innobase/include/srv0srv.h storage/innobase/srv/srv0srv.c === added file 'mysql-test/r/innodb_bug42101-nonzero.result' --- a/mysql-test/r/innodb_bug42101-nonzero.result 1970-01-01 00:00:00 +0000 +++ b/mysql-test/r/innodb_bug42101-nonzero.result 2009-05-19 08:20:28 +0000 @@ -0,0 +1,22 @@ +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 === added file 'mysql-test/r/innodb_bug42101.result' --- a/mysql-test/r/innodb_bug42101.result 1970-01-01 00:00:00 +0000 +++ b/mysql-test/r/innodb_bug42101.result 2009-05-19 08:20:28 +0000 @@ -0,0 +1,18 @@ +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 === added file 'mysql-test/t/innodb_bug42101-nonzero-master.opt' --- a/mysql-test/t/innodb_bug42101-nonzero-master.opt 1970-01-01 00:00:00 +0000 +++ b/mysql-test/t/innodb_bug42101-nonzero-master.opt 2009-05-19 08:20:28 +0000 @@ -0,0 +1 @@ +--innodb_commit_concurrency=1 === added file 'mysql-test/t/innodb_bug42101-nonzero.test' --- a/mysql-test/t/innodb_bug42101-nonzero.test 1970-01-01 00:00:00 +0000 +++ b/mysql-test/t/innodb_bug42101-nonzero.test 2009-05-19 08:20:28 +0000 @@ -0,0 +1,19 @@ +# +# 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; === added file 'mysql-test/t/innodb_bug42101.test' --- a/mysql-test/t/innodb_bug42101.test 1970-01-01 00:00:00 +0000 +++ b/mysql-test/t/innodb_bug42101.test 2009-05-19 08:20:28 +0000 @@ -0,0 +1,17 @@ +# +# 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; === modified file 'storage/innobase/handler/ha_innodb.cc' --- a/storage/innobase/handler/ha_innodb.cc 2009-05-13 10:11:24 +0000 +++ b/storage/innobase/handler/ha_innodb.cc 2009-05-19 08:20:28 +0000 @@ -101,6 +101,7 @@ static long innobase_mirrored_log_groups 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( 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(save) = commit_concurrency + = static_cast(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 @@ retry: 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 "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, === modified file 'storage/innobase/include/srv0srv.h' --- a/storage/innobase/include/srv0srv.h 2009-04-24 11:28:46 +0000 +++ b/storage/innobase/include/srv0srv.h 2009-05-19 08:20:28 +0000 @@ -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; === modified file 'storage/innobase/srv/srv0srv.c' --- a/storage/innobase/srv/srv0srv.c 2009-04-24 11:28:46 +0000 +++ b/storage/innobase/srv/srv0srv.c 2009-05-19 08:20:28 +0000 @@ -285,7 +285,6 @@ computer. Bigger computers need bigger v 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 */