From 3ea0fd9d89da5498f82599d9dc6b84e79d7f9fe3 Mon Sep 17 00:00:00 2001 From: Satya B Date: Tue, 19 May 2009 13:50:28 +0530 Subject: [PATCH] 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 | 22 ++++++++++ mysql-test/r/innodb_bug42101.result | 18 ++++++++ .../t/innodb_bug42101-nonzero-master.opt | 1 + mysql-test/t/innodb_bug42101-nonzero.test | 19 ++++++++ mysql-test/t/innodb_bug42101.test | 17 ++++++++ storage/innobase/handler/ha_innodb.cc | 43 ++++++++++++++++--- storage/innobase/include/srv0srv.h | 1 - storage/innobase/srv/srv0srv.c | 1 - 8 files changed, 115 insertions(+), 7 deletions(-) create mode 100644 mysql-test/r/innodb_bug42101-nonzero.result create mode 100644 mysql-test/r/innodb_bug42101.result create mode 100644 mysql-test/t/innodb_bug42101-nonzero-master.opt create mode 100644 mysql-test/t/innodb_bug42101-nonzero.test create mode 100644 mysql-test/t/innodb_bug42101.test diff --git a/mysql-test/r/innodb_bug42101-nonzero.result b/mysql-test/r/innodb_bug42101-nonzero.result new file mode 100644 index 00000000000..8a14296381c --- /dev/null +++ b/mysql-test/r/innodb_bug42101-nonzero.result @@ -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 diff --git a/mysql-test/r/innodb_bug42101.result b/mysql-test/r/innodb_bug42101.result new file mode 100644 index 00000000000..9a9c8e0ce9b --- /dev/null +++ b/mysql-test/r/innodb_bug42101.result @@ -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 diff --git a/mysql-test/t/innodb_bug42101-nonzero-master.opt b/mysql-test/t/innodb_bug42101-nonzero-master.opt new file mode 100644 index 00000000000..d71dbe17d5b --- /dev/null +++ b/mysql-test/t/innodb_bug42101-nonzero-master.opt @@ -0,0 +1 @@ +--innodb_commit_concurrency=1 diff --git a/mysql-test/t/innodb_bug42101-nonzero.test b/mysql-test/t/innodb_bug42101-nonzero.test new file mode 100644 index 00000000000..c691a234c51 --- /dev/null +++ b/mysql-test/t/innodb_bug42101-nonzero.test @@ -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; diff --git a/mysql-test/t/innodb_bug42101.test b/mysql-test/t/innodb_bug42101.test new file mode 100644 index 00000000000..13d531ecde7 --- /dev/null +++ b/mysql-test/t/innodb_bug42101.test @@ -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; diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index b1c7221f4ec..56e28cf5f14 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/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(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_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, diff --git a/storage/innobase/include/srv0srv.h b/storage/innobase/include/srv0srv.h index 9ff3c225eb0..67144a41d3d 100644 --- a/storage/innobase/include/srv0srv.h +++ b/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; diff --git a/storage/innobase/srv/srv0srv.c b/storage/innobase/srv/srv0srv.c index b8b63052394..71e74ab848b 100644 --- a/storage/innobase/srv/srv0srv.c +++ b/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 */