- developers - lists.mariadb.org

[Maria-developers] Review requested for MDEV-7818
by Kristian Nielsen 09 Jun '15

09 Jun '15

I need someone to review my below fix for MDEV-7818. This involves the FLUSH TABLES WITH READ LOCK command and associated metadata locking, something that I know little about. Monty suggested the basic semantics - that all currently running parallel replication transactions would be allowed to complete before granting the global read lock. But it really needs a review of the actual implementation, by someone who has an understanding of the FTWRL semantics and implementation. Thanks, - Kristian. Kristian Nielsen <knielsen(a)knielsen-hq.org> writes: > revision-id: 0e609daea4796fafca94f7adf796addfc506168b > parent(s): 22b6c297dc0de6b176238ae820bddb5e70f2c8ab > committer: Kristian Nielsen > branch nick: mariadb > timestamp: 2015-06-09 16:01:53 +0200 > message: > > MDEV-7818: Deadlock occurring with parallel replication and FTWRL > > Problem is that FLUSH TABLES WITH READ LOCK first blocks threads from > starting new commits, then waits for running commits to complete. But > in-order parallel replication needs commits to happen in a particular > order, so this can easily deadlock. > > To fix this problem, this patch introduces a way to temporarily pause > the parallel replication worker threads. Before starting FTWRL, we let > all worker threads complete in-progress transactions, and then > wait. Then we proceed to take the global read lock. Once the lock is > obtained, we unpause the worker threads. Now commits are blocked from > starting by the global read lock, so the deadlock will no longer occur. > > --- > sql/mysqld.cc | 3 + > sql/mysqld.h | 3 + > sql/rpl_parallel.cc | 230 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > sql/rpl_parallel.h | 27 ++++-- > sql/sql_parse.cc | 13 +++ > 5 files changed, 262 insertions(+), 14 deletions(-) > > diff --git a/sql/mysqld.cc b/sql/mysqld.cc > index e05c0b6..af8ae2c 100644 > --- a/sql/mysqld.cc > +++ b/sql/mysqld.cc > @@ -9514,6 +9514,9 @@ PSI_stage_info stage_waiting_for_prior_transaction_to_commit= { 0, "Waiting for > PSI_stage_info stage_waiting_for_prior_transaction_to_start_commit= { 0, "Waiting for prior transaction to start commit before starting next transaction", 0}; > PSI_stage_info stage_waiting_for_room_in_worker_thread= { 0, "Waiting for room in worker thread event queue", 0}; > PSI_stage_info stage_waiting_for_workers_idle= { 0, "Waiting for worker threads to be idle", 0}; > +PSI_stage_info stage_waiting_for_ftwrl= { 0, "Waiting due to global read lock", 0}; > +PSI_stage_info stage_waiting_for_ftwrl_threads_to_pause= { 0, "Waiting for worker threads to pause for global read lock", 0}; > +PSI_stage_info stage_waiting_for_rpl_thread_pool= { 0, "Waiting while replication worker thread pool is busy", 0}; > PSI_stage_info stage_master_gtid_wait_primary= { 0, "Waiting in MASTER_GTID_WAIT() (primary waiter)", 0}; > PSI_stage_info stage_master_gtid_wait= { 0, "Waiting in MASTER_GTID_WAIT()", 0}; > PSI_stage_info stage_gtid_wait_other_connection= { 0, "Waiting for other master connection to process GTID received on multiple master connections", 0}; > diff --git a/sql/mysqld.h b/sql/mysqld.h > index 156e7f9..8c953c1 100644 > --- a/sql/mysqld.h > +++ b/sql/mysqld.h > @@ -454,6 +454,9 @@ extern PSI_stage_info stage_waiting_for_prior_transaction_to_commit; > extern PSI_stage_info stage_waiting_for_prior_transaction_to_start_commit; > extern PSI_stage_info stage_waiting_for_room_in_worker_thread; > extern PSI_stage_info stage_waiting_for_workers_idle; > +extern PSI_stage_info stage_waiting_for_ftwrl; > +extern PSI_stage_info stage_waiting_for_ftwrl_threads_to_pause; > +extern PSI_stage_info stage_waiting_for_rpl_thread_pool; > extern PSI_stage_info stage_master_gtid_wait_primary; > extern PSI_stage_info stage_master_gtid_wait; > extern PSI_stage_info stage_gtid_wait_other_connection; > diff --git a/sql/rpl_parallel.cc b/sql/rpl_parallel.cc > index 738d0e5..d90507a 100644 > --- a/sql/rpl_parallel.cc > +++ b/sql/rpl_parallel.cc > @@ -280,6 +280,8 @@ do_gco_wait(rpl_group_info *rgi, group_commit_orderer *gco, > rpl_parallel_entry *entry= rgi->parallel_entry; > uint64 wait_count; > > + mysql_mutex_assert_owner(&entry->LOCK_parallel_entry); > + > if (!gco->installed) > { > group_commit_orderer *prev_gco= gco->prev_gco; > @@ -336,6 +338,159 @@ do_gco_wait(rpl_group_info *rgi, group_commit_orderer *gco, > } > > > +static void > +do_ftwrl_wait(rpl_group_info *rgi, > + bool *did_enter_cond, PSI_stage_info *old_stage) > +{ > + THD *thd= rgi->thd; > + rpl_parallel_entry *entry= rgi->parallel_entry; > + uint64 sub_id= rgi->gtid_sub_id; > + > + mysql_mutex_assert_owner(&entry->LOCK_parallel_entry); > + > + if (unlikely(entry->pause_sub_id > 0) && sub_id > entry->pause_sub_id) > + { > + thd->ENTER_COND(&entry->COND_parallel_entry, &entry->LOCK_parallel_entry, > + &stage_waiting_for_ftwrl, old_stage); > + *did_enter_cond= true; > + do > + { > + if (entry->force_abort || rgi->worker_error) > + break; > + if (thd->check_killed()) > + { > + thd->send_kill_message(); > + slave_output_error_info(rgi, thd); > + signal_error_to_sql_driver_thread(thd, rgi, 1); > + break; > + } > + mysql_cond_wait(&entry->COND_parallel_entry, &entry->LOCK_parallel_entry); > + } while (entry->pause_sub_id > 0 && sub_id > entry->pause_sub_id); > + } > + > + if (sub_id > entry->largest_started_sub_id) > + entry->largest_started_sub_id= sub_id; > +} > + > + > +static void > +pool_mark_busy(rpl_parallel_thread_pool *pool) > +{ > + mysql_mutex_assert_owner(&pool->LOCK_rpl_thread_pool); > + DBUG_ASSERT(!pool->busy); > + pool->busy= true; > +} > + > + > +static void > +pool_mark_not_busy(rpl_parallel_thread_pool *pool) > +{ > + mysql_mutex_assert_owner(&pool->LOCK_rpl_thread_pool); > + DBUG_ASSERT(pool->busy); > + pool->busy= false; > + mysql_cond_broadcast(&pool->COND_rpl_thread_pool); > +} > + > + > +void > +rpl_unpause_for_ftwrl(THD *thd) > +{ > + uint32 i; > + rpl_parallel_thread_pool *pool= &global_rpl_thread_pool; > + > + DBUG_ASSERT(pool->busy); > + > + for (i= 0; i < pool->count; ++i) > + { > + rpl_parallel_entry *e; > + rpl_parallel_thread *rpt= pool->threads[i]; > + > + mysql_mutex_lock(&rpt->LOCK_rpl_thread); > + if (!rpt->current_owner) > + { > + mysql_mutex_unlock(&rpt->LOCK_rpl_thread); > + continue; > + } > + e= rpt->current_entry; > + mysql_mutex_lock(&e->LOCK_parallel_entry); > + mysql_mutex_unlock(&rpt->LOCK_rpl_thread); > + e->pause_sub_id= 0; > + mysql_cond_broadcast(&e->COND_parallel_entry); > + } > + > + mysql_mutex_lock(&pool->LOCK_rpl_thread_pool); > + pool_mark_not_busy(pool); > + mysql_cond_broadcast(&pool->COND_rpl_thread_pool); > + mysql_mutex_unlock(&pool->LOCK_rpl_thread_pool); > +} > + > + > +/* > + . > + > + Note: in case of error return, unpause_for_ftwrl() must _not_ be called. > +*/ > +int > +rpl_pause_for_ftwrl(THD *thd) > +{ > + uint32 i; > + rpl_parallel_thread_pool *pool= &global_rpl_thread_pool; > + int err= 0; > + > + /* > + While the count_pending_pause_for_ftwrl counter is non-zero, the pool > + cannot be shutdown/resized, so threads are guaranteed to not disappear. > + > + This is required to safely be able to access the individual threads below. > + (We cannot lock an individual thread while holding LOCK_rpl_thread_pool, > + as this can deadlock against release_thread()). > + */ > + mysql_mutex_lock(&pool->LOCK_rpl_thread_pool); > + pool_mark_busy(pool); > + mysql_mutex_unlock(&pool->LOCK_rpl_thread_pool); > + > + for (i= 0; i < pool->count; ++i) > + { > + PSI_stage_info old_stage; > + rpl_parallel_entry *e; > + rpl_parallel_thread *rpt= pool->threads[i]; > + > + mysql_mutex_lock(&rpt->LOCK_rpl_thread); > + if (!rpt->current_owner) > + { > + mysql_mutex_unlock(&rpt->LOCK_rpl_thread); > + continue; > + } > + e= rpt->current_entry; > + mysql_mutex_lock(&e->LOCK_parallel_entry); > + mysql_mutex_unlock(&rpt->LOCK_rpl_thread); > + ++e->need_sub_id_signal; > + if (!e->pause_sub_id) > + e->pause_sub_id= e->largest_started_sub_id; > + thd->ENTER_COND(&e->COND_parallel_entry, &e->LOCK_parallel_entry, > + &stage_waiting_for_ftwrl_threads_to_pause, &old_stage); > + while (e->last_committed_sub_id < e->pause_sub_id && !err) > + { > + if (thd->check_killed()) > + { > + thd->send_kill_message(); > + err= 1; > + break; > + } > + mysql_cond_wait(&e->COND_parallel_entry, &e->LOCK_parallel_entry); > + }; > + --e->need_sub_id_signal; > + thd->EXIT_COND(&old_stage); > + if (err) > + break; > + } > + > + if (err) > + rpl_unpause_for_ftwrl(thd); > + return err; > +} > + > + > #ifndef DBUG_OFF > static int > dbug_simulate_tmp_error(rpl_group_info *rgi, THD *thd) > @@ -856,6 +1011,9 @@ handle_rpl_parallel_thread(void *arg) > > if (unlikely(entry->stop_on_error_sub_id <= rgi->wait_commit_sub_id)) > skip_event_group= true; > + if (likely(!skip_event_group)) > + do_ftwrl_wait(rgi, &did_enter_cond, &old_stage); > + > register_wait_for_prior_event_group_commit(rgi, entry); > > unlock_or_exit_cond(thd, &entry->LOCK_parallel_entry, > @@ -1058,6 +1216,47 @@ rpl_parallel_change_thread_count(rpl_parallel_thread_pool *pool, > rpl_parallel_thread **new_list= NULL; > rpl_parallel_thread *new_free_list= NULL; > rpl_parallel_thread *rpt_array= NULL; > + THD *thd; > + PSI_stage_info old_stage; > + int res; > + > + /* > + Wait here while the queue is busy. This is done to make FLUSH TABLES WITH > + READ LOCK work correctly, without incuring extra locking penalties in > + normal operation. FLUSH TABLES WITH READ LOCK needs to lock threads in the > + thread pool, and for this we need to make sure the pool will not go away > + during the operation. The LOCK_rpl_thread_pool is not suitable for > + this. It is taken by release_thread() while holding LOCK_rpl_thread; so it > + must be released before locking any LOCK_rpl_thread lock, or a deadlock > + can occur. > + > + So we protect the infrequent operations of FLUSH TABLES WITH READ LOCK and > + pool size changes with this condition wait. > + */ > + thd= current_thd; > + res= 0; > + mysql_mutex_lock(&pool->LOCK_rpl_thread_pool); > + if (thd) > + thd->ENTER_COND(&pool->COND_rpl_thread_pool, &pool->LOCK_rpl_thread_pool, > + &stage_waiting_for_rpl_thread_pool, &old_stage); > + while (pool->busy) > + { > + if (thd->check_killed()) > + { > + thd->send_kill_message(); > + res= 1; > + break; > + } > + mysql_cond_wait(&pool->COND_rpl_thread_pool, &pool->LOCK_rpl_thread_pool); > + } > + if (!res) > + pool_mark_busy(pool); > + if (thd) > + thd->EXIT_COND(&old_stage); > + else > + mysql_mutex_unlock(&pool->LOCK_rpl_thread_pool); > + if (res) > + return res; > > /* > Allocate the new list of threads up-front. > @@ -1106,7 +1305,14 @@ rpl_parallel_change_thread_count(rpl_parallel_thread_pool *pool, > */ > for (i= 0; i < pool->count; ++i) > { > - rpl_parallel_thread *rpt= pool->get_thread(NULL, NULL); > + rpl_parallel_thread *rpt; > + > + mysql_mutex_lock(&pool->LOCK_rpl_thread_pool); > + while ((rpt= pool->free_list) == NULL) > + mysql_cond_wait(&pool->COND_rpl_thread_pool, &pool->LOCK_rpl_thread_pool); > + pool->free_list= rpt->next; > + mysql_mutex_unlock(&pool->LOCK_rpl_thread_pool); > + mysql_mutex_lock(&rpt->LOCK_rpl_thread); > rpt->stop= true; > mysql_cond_signal(&rpt->COND_rpl_thread); > mysql_mutex_unlock(&rpt->LOCK_rpl_thread); > @@ -1157,7 +1363,7 @@ rpl_parallel_change_thread_count(rpl_parallel_thread_pool *pool, > } > > mysql_mutex_lock(&pool->LOCK_rpl_thread_pool); > - mysql_cond_broadcast(&pool->COND_rpl_thread_pool); > + pool_mark_not_busy(pool); > mysql_mutex_unlock(&pool->LOCK_rpl_thread_pool); > > return 0; > @@ -1182,6 +1388,9 @@ rpl_parallel_change_thread_count(rpl_parallel_thread_pool *pool, > } > my_free(new_list); > } > + mysql_mutex_lock(&pool->LOCK_rpl_thread_pool); > + pool_mark_not_busy(pool); > + mysql_mutex_unlock(&pool->LOCK_rpl_thread_pool); > return 1; > } > > @@ -1445,7 +1654,7 @@ rpl_parallel_thread::loc_free_gco(group_commit_orderer *gco) > > > rpl_parallel_thread_pool::rpl_parallel_thread_pool() > - : count(0), threads(0), free_list(0), inited(false) > + : threads(0), free_list(0), count(0), inited(false), busy(false) > { > } > > @@ -1453,9 +1662,10 @@ rpl_parallel_thread_pool::rpl_parallel_thread_pool() > int > rpl_parallel_thread_pool::init(uint32 size) > { > - count= 0; > threads= NULL; > free_list= NULL; > + count= 0; > + busy= false; > > mysql_mutex_init(key_LOCK_rpl_thread_pool, &LOCK_rpl_thread_pool, > MY_MUTEX_INIT_SLOW); > @@ -1496,8 +1706,14 @@ rpl_parallel_thread_pool::get_thread(rpl_parallel_thread **owner, > rpl_parallel_thread *rpt; > > mysql_mutex_lock(&LOCK_rpl_thread_pool); > - while ((rpt= free_list) == NULL) > + for (;;) > + { > + while (unlikely(busy)) > + mysql_cond_wait(&COND_rpl_thread_pool, &LOCK_rpl_thread_pool); > + if ((rpt= free_list) != NULL) > + break; > mysql_cond_wait(&COND_rpl_thread_pool, &LOCK_rpl_thread_pool); > + } > free_list= rpt->next; > mysql_mutex_unlock(&LOCK_rpl_thread_pool); > mysql_mutex_lock(&rpt->LOCK_rpl_thread); > @@ -1908,7 +2124,7 @@ rpl_parallel::wait_for_workers_idle(THD *thd) > > e= (struct rpl_parallel_entry *)my_hash_element(&domain_hash, i); > mysql_mutex_lock(&e->LOCK_parallel_entry); > - e->need_sub_id_signal= true; > + ++e->need_sub_id_signal; > thd->ENTER_COND(&e->COND_parallel_entry, &e->LOCK_parallel_entry, > &stage_waiting_for_workers_idle, &old_stage); > while (e->current_sub_id > e->last_committed_sub_id) > @@ -1921,7 +2137,7 @@ rpl_parallel::wait_for_workers_idle(THD *thd) > } > mysql_cond_wait(&e->COND_parallel_entry, &e->LOCK_parallel_entry); > } > - e->need_sub_id_signal= false; > + --e->need_sub_id_signal; > thd->EXIT_COND(&old_stage); > if (err) > return err; > diff --git a/sql/rpl_parallel.h b/sql/rpl_parallel.h > index 04f8706..77a4005 100644 > --- a/sql/rpl_parallel.h > +++ b/sql/rpl_parallel.h > @@ -199,12 +199,18 @@ struct rpl_parallel_thread { > > > struct rpl_parallel_thread_pool { > - uint32 count; > struct rpl_parallel_thread **threads; > struct rpl_parallel_thread *free_list; > mysql_mutex_t LOCK_rpl_thread_pool; > mysql_cond_t COND_rpl_thread_pool; > + uint32 count; > bool inited; > + /* > + While FTWRL runs, this counter is incremented to make SQL thread or > + STOP/START slave not try to start new activity while that operation > + is in progress. > + */ > + bool busy; > > rpl_parallel_thread_pool(); > int init(uint32 size); > @@ -219,6 +225,12 @@ struct rpl_parallel_entry { > mysql_mutex_t LOCK_parallel_entry; > mysql_cond_t COND_parallel_entry; > uint32 domain_id; > + /* > + Incremented by wait_for_workers_idle() and rpl_pause_for_ftwrl() to show > + that they are waiting, so that finish_event_group knows to signal them > + when last_committed_sub_id is increased. > + */ > + uint32 need_sub_id_signal; > uint64 last_commit_id; > bool active; > /* > @@ -228,12 +240,6 @@ struct rpl_parallel_entry { > */ > bool force_abort; > /* > - Set in wait_for_workers_idle() to show that it is waiting, so that > - finish_event_group knows to signal it when last_committed_sub_id is > - increased. > - */ > - bool need_sub_id_signal; > - /* > At STOP SLAVE (force_abort=true), we do not want to process all events in > the queue (which could unnecessarily delay stop, if a lot of events happen > to be queued). The stop_count provides a safe point at which to stop, so > @@ -291,6 +297,11 @@ struct rpl_parallel_entry { > The value is ULONGLONG_MAX when no error occured. > */ > uint64 stop_on_error_sub_id; > + /* > + During FLUSH TABLES WITH READ LOCK, transactions with sub_id larger than > + this value must not start, but wait until the global read lock is released. > + */ > + uint64 pause_sub_id; > /* Total count of event groups queued so far. */ > uint64 count_queued_event_groups; > /* > @@ -331,5 +342,7 @@ extern struct rpl_parallel_thread_pool global_rpl_thread_pool; > extern int rpl_parallel_activate_pool(rpl_parallel_thread_pool *pool); > extern int rpl_parallel_inactivate_pool(rpl_parallel_thread_pool *pool); > extern bool process_gtid_for_restart_pos(Relay_log_info *rli, rpl_gtid *gtid); > +extern int rpl_pause_for_ftwrl(THD *thd); > +extern void rpl_unpause_for_ftwrl(THD *thd); > > #endif /* RPL_PARALLEL_H */ > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index 5635e9a..ca6b11e 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -4271,6 +4271,17 @@ case SQLCOM_PREPARE: > break; > } > > + if (lex->type & REFRESH_READ_LOCK) > + { > + /* > + We need to pause any parallel replication slave workers during FLUSH > + TABLES WITH READ LOCK. Otherwise we might cause a deadlock, as > + worker threads eun run in arbitrary order but need to commit in a > + specific given order. > + */ > + if (rpl_pause_for_ftwrl(thd)) > + goto error; > + } > /* > reload_acl_and_cache() will tell us if we are allowed to write to the > binlog or not. > @@ -4301,6 +4312,8 @@ case SQLCOM_PREPARE: > if (!res) > my_ok(thd); > } > + if (lex->type & REFRESH_READ_LOCK) > + rpl_unpause_for_ftwrl(thd); > > break; > } > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 0

Re: [Maria-developers] b8e26b0: MDEV-363 - Server crashes in intern_plugin_lock on concurrent installing
by Sergei Golubchik 09 Jun '15

09 Jun '15

Hi, Sergey! On Jun 09, svoj(a)mariadb.org wrote: > revision-id: b8e26b04d4ac22e29a78bca68e101176b6ef507a > parent(s): 08b2e1ae73f6fe055391bf8437b6dff1a889c49c > committer: Sergey Vojtovich > branch nick: mariadb > timestamp: 2015-06-09 13:50:43 +0400 > message: > > MDEV-363 - Server crashes in intern_plugin_lock on concurrent installing > semisync plugin and setting rpl_semi_sync_master_enabled > > There was race condition between INSTALL PLUGIN and SET. It was caused by a > gap in INSTALL PLUGIN when plugin variables were registered but not fully > initialized. Accessing such variables concurrently may reference uninitialized > memory, specifically sys_var_pluginvar::plugin. > > Fixed by initializing sys_var_pluginvar::plugin early, before variable is > registered. ok to push! Regards, Sergei

1 0

[Maria-developers] MDEV-7268 Column of table cannot be converted from type 'decimal(0, ?)' to type ' 'decimal(10, 7)'
by Alexander Barkov 09 Jun '15

09 Jun '15

Hi Sergei, Please review a patch for mdev-7268. Thanks.

2 1

Re: [Maria-developers] 1ed4683: MDEV-8260 : Issues related to concurrent CTAS
by Sergei Golubchik 09 Jun '15

09 Jun '15

Hi, Nirbhay! On Jun 02, Nirbhay Choubey wrote: > revision-id: 1ed46831ed3a1c842b45f7819edd48a233080ce4 > parent(s): 6d5b723bdc3e04978619b9673fca266e0426916f > committer: Nirbhay Choubey > branch nick: 10.0-galera.ctas > timestamp: 2015-06-02 17:59:32 -0400 > message: > > MDEV-8260 : Issues related to concurrent CTAS > > * Wait for aborted thd (victim) to release MDL locks > * Skip aborting an already aborted thd > * Defer setting OK status in case of CTAS > * Minor cosmetic changes > * Added a test case So, I'm only reviewing non-wsrep part, ok? > @@ -3735,24 +3757,16 @@ bool select_insert::send_eof() > table->file->print_error(error,MYF(0)); > DBUG_RETURN(1); > } > - char buff[160]; > - if (info.ignore) > - sprintf(buff, ER(ER_INSERT_INFO), (ulong) info.records, > - (ulong) (info.records - info.copied), > - (long) thd->get_stmt_da()->current_statement_warn_count()); > - else > - sprintf(buff, ER(ER_INSERT_INFO), (ulong) info.records, > - (ulong) (info.deleted+info.updated), > - (long) thd->get_stmt_da()->current_statement_warn_count()); > - row_count= info.copied + info.deleted + > - ((thd->client_capabilities & CLIENT_FOUND_ROWS) ? > - info.touched : info.updated); > - id= (thd->first_successful_insert_id_in_cur_stmt > 0) ? > - thd->first_successful_insert_id_in_cur_stmt : > - (thd->arg_of_last_insert_id_function ? > - thd->first_successful_insert_id_in_prev_stmt : > - (info.copied ? autoinc_value_of_last_inserted_row : 0)); > - ::my_ok(thd, row_count, id, buff); > + > + /* > + Avoid setting OK status for CREATE .. SELECT as it may fail during commit. > + */ > + if (!((thd->lex->sql_command == SQLCOM_CREATE_TABLE) && > + ((&thd->lex->select_lex)->item_list.elements))) { > + prepare_ok_info(thd); > + ::my_ok(thd, ok.row_count, ok.id, ok.message); > + } > + > DBUG_RETURN(0); > } I think we've discussed a slightly different fix, where the parent class (select_insert) wouldn't need to check thd->lex->sql_command. Like, create two methods select_insert::prepare_eof select_insert::send_ok_packet the first method will have everything of the current select_insert::send_eof but without ::my_ok part. Then select_insert::send_eof() will be just bool select_insert::send_eof() { return prepare_eof() || send_ok_packet(); } and select_create::send_eof() will do prepare_eof(), commit and other stuff, send_ok_packet(). In this case you won't need to test for thd->lex->sql_command in the parent class. And you won't need prepare_ok_info(), it'll be inside send_ok_packet(). Regards, Sergei

2 1

Re: [Maria-developers] 87088b9: MDEV-8211 plugins.server_audit fails sporadically in buildbot.
by Sergei Golubchik 08 Jun '15

08 Jun '15

Hi, Holyfoot! On Jun 08, holyfoot(a)askmonty.org wrote: > revision-id: 87088b91f7dfa49a1946454d347d823dab643569 > parent(s): 96b37035e4045a27f64810c2d946bad2d1453a80 > committer: Alexey Botchkov > branch nick: server > timestamp: 2015-06-08 21:44:13 +0500 > message: > > MDEV-8211 plugins.server_audit fails sporadically in buildbot. > This test also should be fixed - delay added so the connection > event doesn't happen before the query. > > --- > mysql-test/suite/plugins/t/thread_pool_server_audit.test | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mysql-test/suite/plugins/t/thread_pool_server_audit.test b/mysql-test/suite/plugins/t/thread_pool_server_audit.test > index 1a2d729..2c749e2 100644 > --- a/mysql-test/suite/plugins/t/thread_pool_server_audit.test > +++ b/mysql-test/suite/plugins/t/thread_pool_server_audit.test > @@ -13,6 +13,7 @@ set global server_audit_incl_users=null; > set global server_audit_file_path='server_audit.log'; > set global server_audit_output_type=file; > set global server_audit_logging=on; > +--sleep 2 > connect (con1,localhost,root,,mysql); > connection default; > disconnect con1; This is not a good idea. First, delays make mysql-test slower. And most often - unnecessary slower, as the test succeeds without a delay too. But also delays are unreliable, on a slow box two seconds may be not enough. Don't use delays, please, in this case you can wait for the query to complete before opening a new connection. Regards, Sergei

1 0

Re: [Maria-developers] [Commits] 5ccbfa0: MDEV-7445: Server crash with Signal 6
by Sergey Petrunia 08 Jun '15

08 Jun '15

On Thu, Apr 23, 2015 at 07:11:09PM +0200, sanja(a)mariadb.com wrote: > revision-id: 5ccbfa02d4da91c3f9080dc1b1627d19cf40eccd > parent(s): c7efc0315b51f214b6423ae4dcb4ddeb1b14e91a > committer: Oleksandr Byelkin > branch nick: server > timestamp: 2015-04-23 19:11:06 +0200 > message: > > MDEV-7445: Server crash with Signal 6 > > Problem was in rewriting left expression which had 2 references on it. Solved with making subselect reference main. > > Item_in_optimized can have not Item_in_subselect reference in left part so type casting with no check is dangerous. > > Item::cols() should be checked after Item::fix_fields(). > > --- ... > diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc > index 998cb1c..51a38f0 100644 > --- a/sql/item_cmpfunc.cc > +++ b/sql/item_cmpfunc.cc > @@ -1442,9 +1442,25 @@ bool Item_in_optimizer::eval_not_null_tables(uchar *opt_arg) > bool Item_in_optimizer::fix_left(THD *thd, Item **ref) > { > DBUG_ENTER("Item_in_optimizer::fix_left"); > - if ((!args[0]->fixed && args[0]->fix_fields(thd, args)) || > - (!cache && !(cache= Item_cache::get_cache(args[0])))) > + Item **ref0= args; > + if (args[1]->type() == Item::SUBSELECT_ITEM && > + ((Item_subselect *)args[1])->is_in_predicate()) > + { > + /* > + left_expr->fix_fields() may cause left_expr to be substituted for > + another item. (e.g. an Item_field may be changed into Item_ref). This > + transformation is undone at the end of statement execution (e.g. the > + Item_ref is deleted). However, Item_in_optimizer::args[0] may keep > + the pointer to the post-transformation item. Because of that, on the > + next execution we need to copy args[1]->left_expr again. > + */ > + ref0= &(((Item_in_subselect *)args[1])->left_expr); > + args[0]= ref0[0]; > + } > + if ((!args[0]->fixed && args[0]->fix_fields(thd, ref0)) || > + (!cache && !(cache= Item_cache::get_cache(ref0[0])))) > DBUG_RETURN(1); > + args[0]= ref0[0]; > DBUG_PRINT("info", ("actual fix fields")); > > cache->setup(args[0]); > @@ -1500,6 +1516,16 @@ bool Item_in_optimizer::fix_left(THD *thd, Item **ref) > bool Item_in_optimizer::fix_fields(THD *thd, Item **ref) > { > DBUG_ASSERT(fixed == 0); > + Item_subselect *sub= 0; > + uint col; > + > + /* > + MAX/MIN optimization can convert the subquery into > + expr + Item_singlerow_subselect > + */ (*) > + if (args[1]->type() == Item::SUBSELECT_ITEM) > + sub= (Item_subselect *)args[1]; > + I was looking for the case where the above if statement is not taken. Eventually I have applied this patch, the patch after it (e540d023e2ec6f37efc9ab695ccdfd4a6744ad64) to get the test cases, then added this at line (*): DBUG_ASSERT(args[1]->type() == Item::SUBSELECT_ITEM); and ran the entire main test suite, with and without --ps-protocol. It worked. Can you add test coverage for this case? > if (fix_left(thd, ref)) > return TRUE; > if (args[0]->maybe_null) > @@ -1507,10 +1533,10 @@ bool Item_in_optimizer::fix_fields(THD *thd, Item **ref) > > if (!args[1]->fixed && args[1]->fix_fields(thd, args+1)) > return TRUE; > - Item_in_subselect * sub= (Item_in_subselect *)args[1]; > - if (args[0]->cols() != sub->engine->cols()) > + if ((sub && ((col= args[0]->cols()) != sub->engine->cols())) || > + (!sub && (args[1]->cols() != (col= 1)))) > { > - my_error(ER_OPERAND_COLUMNS, MYF(0), args[0]->cols()); > + my_error(ER_OPERAND_COLUMNS, MYF(0), col); > return TRUE; > } > if (args[1]->maybe_null) ... BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 1

Re: [Maria-developers] [Commits] 9f8a458: MDEV-7846: Server crashes in Item_subselect::fix_fields or fails with Thread stack overrun
by Sergey Petrunia 08 Jun '15

08 Jun '15

Hi Sanja, It is very difficult to understand this patch. What is the exact meaning of left_expr_orig? It is declared but has no comments. It is interesting that convert_subq_to_sj has got new code to handle scalar comparison case ( foo IN (SELECT bar ...)), while there is no handling for tuple comparison ( (foo1,foo2) IN (SELECT bar1,bar2 ..)). Is this intentional? On Thu, Apr 23, 2015 at 08:09:00PM +0200, sanja(a)mariadb.com wrote: > revision-id: 9f8a458fb2d07298810bb5d9824ce728017bfb32 > parent(s): e540d023e2ec6f37efc9ab695ccdfd4a6744ad64 > committer: Oleksandr Byelkin > branch nick: server > timestamp: 2015-04-23 20:08:57 +0200 > message: > > MDEV-7846: Server crashes in Item_subselect::fix_fields or fails with Thread stack overrun > > Substitute into transformed subselects original left expression and than register its change in case it was substituted. > > --- > mysql-test/r/subselect.result | 28 +++++++++++++++++++++++++ > mysql-test/r/subselect_no_mat.result | 28 +++++++++++++++++++++++++ > mysql-test/r/subselect_no_opts.result | 28 +++++++++++++++++++++++++ > mysql-test/r/subselect_no_scache.result | 28 +++++++++++++++++++++++++ > mysql-test/r/subselect_no_semijoin.result | 28 +++++++++++++++++++++++++ > mysql-test/t/subselect.test | 34 +++++++++++++++++++++++++++++++ > sql/item_cmpfunc.cc | 8 ++++---- > sql/item_subselect.cc | 8 +++----- > sql/item_subselect.h | 1 + > sql/opt_subselect.cc | 4 +++- > 10 files changed, 185 insertions(+), 10 deletions(-) > > diff --git a/mysql-test/r/subselect.result b/mysql-test/r/subselect.result > index 0ab2d48..cf52ba2 100644 > --- a/mysql-test/r/subselect.result > +++ b/mysql-test/r/subselect.result > @@ -7053,3 +7053,31 @@ WHERE h.host in (SELECT host FROM mysql.user) > ) AS sq > FROM mysql.host h GROUP BY h.host; > sq > +# > +# MDEV-7846:Server crashes in Item_subselect::fix > +#_fields or fails with Thread stack overrun > +# > +CREATE TABLE t1 (column1 INT) ENGINE=MyISAM; > +INSERT INTO t1 VALUES (3),(9); > +CREATE TABLE t2 (column2 INT) ENGINE=MyISAM; > +INSERT INTO t2 VALUES (1),(4); > +CREATE TABLE t3 (column3 INT) ENGINE=MyISAM; > +INSERT INTO t3 VALUES (6),(8); > +CREATE TABLE t4 (column4 INT) ENGINE=MyISAM; > +INSERT INTO t4 VALUES (2),(5); > +PREPARE stmt FROM " > +SELECT ( > + SELECT MAX( table1.column1 ) AS field1 > + FROM t1 AS table1 > + WHERE table3.column3 IN ( SELECT table2.column2 AS field2 FROM t2 AS table2 ) > +) AS sq > +FROM t3 AS table3, t4 AS table4 GROUP BY sq > +"; > +EXECUTE stmt; > +sq > +NULL > +EXECUTE stmt; > +sq > +NULL > +deallocate prepare stmt; > +drop table t1,t2,t3,t4; > diff --git a/mysql-test/r/subselect_no_mat.result b/mysql-test/r/subselect_no_mat.result > index 5780351..73a69e9 100644 > --- a/mysql-test/r/subselect_no_mat.result > +++ b/mysql-test/r/subselect_no_mat.result > @@ -7050,6 +7050,34 @@ WHERE h.host in (SELECT host FROM mysql.user) > ) AS sq > FROM mysql.host h GROUP BY h.host; > sq > +# > +# MDEV-7846:Server crashes in Item_subselect::fix > +#_fields or fails with Thread stack overrun > +# > +CREATE TABLE t1 (column1 INT) ENGINE=MyISAM; > +INSERT INTO t1 VALUES (3),(9); > +CREATE TABLE t2 (column2 INT) ENGINE=MyISAM; > +INSERT INTO t2 VALUES (1),(4); > +CREATE TABLE t3 (column3 INT) ENGINE=MyISAM; > +INSERT INTO t3 VALUES (6),(8); > +CREATE TABLE t4 (column4 INT) ENGINE=MyISAM; > +INSERT INTO t4 VALUES (2),(5); > +PREPARE stmt FROM " > +SELECT ( > + SELECT MAX( table1.column1 ) AS field1 > + FROM t1 AS table1 > + WHERE table3.column3 IN ( SELECT table2.column2 AS field2 FROM t2 AS table2 ) > +) AS sq > +FROM t3 AS table3, t4 AS table4 GROUP BY sq > +"; > +EXECUTE stmt; > +sq > +NULL > +EXECUTE stmt; > +sq > +NULL > +deallocate prepare stmt; > +drop table t1,t2,t3,t4; > set optimizer_switch=default; > select @@optimizer_switch like '%materialization=on%'; > @@optimizer_switch like '%materialization=on%' > diff --git a/mysql-test/r/subselect_no_opts.result b/mysql-test/r/subselect_no_opts.result > index 4bea029..1512e39 100644 > --- a/mysql-test/r/subselect_no_opts.result > +++ b/mysql-test/r/subselect_no_opts.result > @@ -7048,4 +7048,32 @@ WHERE h.host in (SELECT host FROM mysql.user) > ) AS sq > FROM mysql.host h GROUP BY h.host; > sq > +# > +# MDEV-7846:Server crashes in Item_subselect::fix > +#_fields or fails with Thread stack overrun > +# > +CREATE TABLE t1 (column1 INT) ENGINE=MyISAM; > +INSERT INTO t1 VALUES (3),(9); > +CREATE TABLE t2 (column2 INT) ENGINE=MyISAM; > +INSERT INTO t2 VALUES (1),(4); > +CREATE TABLE t3 (column3 INT) ENGINE=MyISAM; > +INSERT INTO t3 VALUES (6),(8); > +CREATE TABLE t4 (column4 INT) ENGINE=MyISAM; > +INSERT INTO t4 VALUES (2),(5); > +PREPARE stmt FROM " > +SELECT ( > + SELECT MAX( table1.column1 ) AS field1 > + FROM t1 AS table1 > + WHERE table3.column3 IN ( SELECT table2.column2 AS field2 FROM t2 AS table2 ) > +) AS sq > +FROM t3 AS table3, t4 AS table4 GROUP BY sq > +"; > +EXECUTE stmt; > +sq > +NULL > +EXECUTE stmt; > +sq > +NULL > +deallocate prepare stmt; > +drop table t1,t2,t3,t4; > set @optimizer_switch_for_subselect_test=null; > diff --git a/mysql-test/r/subselect_no_scache.result b/mysql-test/r/subselect_no_scache.result > index fdb3b12..26cea1f 100644 > --- a/mysql-test/r/subselect_no_scache.result > +++ b/mysql-test/r/subselect_no_scache.result > @@ -7059,6 +7059,34 @@ WHERE h.host in (SELECT host FROM mysql.user) > ) AS sq > FROM mysql.host h GROUP BY h.host; > sq > +# > +# MDEV-7846:Server crashes in Item_subselect::fix > +#_fields or fails with Thread stack overrun > +# > +CREATE TABLE t1 (column1 INT) ENGINE=MyISAM; > +INSERT INTO t1 VALUES (3),(9); > +CREATE TABLE t2 (column2 INT) ENGINE=MyISAM; > +INSERT INTO t2 VALUES (1),(4); > +CREATE TABLE t3 (column3 INT) ENGINE=MyISAM; > +INSERT INTO t3 VALUES (6),(8); > +CREATE TABLE t4 (column4 INT) ENGINE=MyISAM; > +INSERT INTO t4 VALUES (2),(5); > +PREPARE stmt FROM " > +SELECT ( > + SELECT MAX( table1.column1 ) AS field1 > + FROM t1 AS table1 > + WHERE table3.column3 IN ( SELECT table2.column2 AS field2 FROM t2 AS table2 ) > +) AS sq > +FROM t3 AS table3, t4 AS table4 GROUP BY sq > +"; > +EXECUTE stmt; > +sq > +NULL > +EXECUTE stmt; > +sq > +NULL > +deallocate prepare stmt; > +drop table t1,t2,t3,t4; > set optimizer_switch=default; > select @@optimizer_switch like '%subquery_cache=on%'; > @@optimizer_switch like '%subquery_cache=on%' > diff --git a/mysql-test/r/subselect_no_semijoin.result b/mysql-test/r/subselect_no_semijoin.result > index cb6d35d..4c6f037 100644 > --- a/mysql-test/r/subselect_no_semijoin.result > +++ b/mysql-test/r/subselect_no_semijoin.result > @@ -7048,5 +7048,33 @@ WHERE h.host in (SELECT host FROM mysql.user) > ) AS sq > FROM mysql.host h GROUP BY h.host; > sq > +# > +# MDEV-7846:Server crashes in Item_subselect::fix > +#_fields or fails with Thread stack overrun > +# > +CREATE TABLE t1 (column1 INT) ENGINE=MyISAM; > +INSERT INTO t1 VALUES (3),(9); > +CREATE TABLE t2 (column2 INT) ENGINE=MyISAM; > +INSERT INTO t2 VALUES (1),(4); > +CREATE TABLE t3 (column3 INT) ENGINE=MyISAM; > +INSERT INTO t3 VALUES (6),(8); > +CREATE TABLE t4 (column4 INT) ENGINE=MyISAM; > +INSERT INTO t4 VALUES (2),(5); > +PREPARE stmt FROM " > +SELECT ( > + SELECT MAX( table1.column1 ) AS field1 > + FROM t1 AS table1 > + WHERE table3.column3 IN ( SELECT table2.column2 AS field2 FROM t2 AS table2 ) > +) AS sq > +FROM t3 AS table3, t4 AS table4 GROUP BY sq > +"; > +EXECUTE stmt; > +sq > +NULL > +EXECUTE stmt; > +sq > +NULL > +deallocate prepare stmt; > +drop table t1,t2,t3,t4; > set @optimizer_switch_for_subselect_test=null; > set @join_cache_level_for_subselect_test=NULL; > diff --git a/mysql-test/t/subselect.test b/mysql-test/t/subselect.test > index 3eb056d..00ab48b 100644 > --- a/mysql-test/t/subselect.test > +++ b/mysql-test/t/subselect.test > @@ -5930,3 +5930,37 @@ SELECT > WHERE h.host in (SELECT host FROM mysql.user) > ) AS sq > FROM mysql.host h GROUP BY h.host; > + > + > +--echo # > +--echo # MDEV-7846:Server crashes in Item_subselect::fix > +--echo #_fields or fails with Thread stack overrun > +--echo # > +CREATE TABLE t1 (column1 INT) ENGINE=MyISAM; > +INSERT INTO t1 VALUES (3),(9); > + > +CREATE TABLE t2 (column2 INT) ENGINE=MyISAM; > + > +INSERT INTO t2 VALUES (1),(4); > + > +CREATE TABLE t3 (column3 INT) ENGINE=MyISAM; > +INSERT INTO t3 VALUES (6),(8); > + > +CREATE TABLE t4 (column4 INT) ENGINE=MyISAM; > +INSERT INTO t4 VALUES (2),(5); > + > + > +PREPARE stmt FROM " > +SELECT ( > + SELECT MAX( table1.column1 ) AS field1 > + FROM t1 AS table1 > + WHERE table3.column3 IN ( SELECT table2.column2 AS field2 FROM t2 AS table2 ) > +) AS sq > +FROM t3 AS table3, t4 AS table4 GROUP BY sq > +"; > + > +EXECUTE stmt; > +EXECUTE stmt; > + > +deallocate prepare stmt; > +drop table t1,t2,t3,t4; > diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc > index 51a38f0..29fff8e 100644 > --- a/sql/item_cmpfunc.cc > +++ b/sql/item_cmpfunc.cc > @@ -1455,12 +1455,12 @@ bool Item_in_optimizer::fix_left(THD *thd, Item **ref) > next execution we need to copy args[1]->left_expr again. > */ > ref0= &(((Item_in_subselect *)args[1])->left_expr); > - args[0]= ref0[0]; > } > - if ((!args[0]->fixed && args[0]->fix_fields(thd, ref0)) || > - (!cache && !(cache= Item_cache::get_cache(ref0[0])))) > + if ((!(*ref0)->fixed && (*ref0)->fix_fields(thd, ref0)) || > + (!cache && !(cache= Item_cache::get_cache(*ref0)))) > DBUG_RETURN(1); > - args[0]= ref0[0]; > + if (args[0] != (*ref0)) > + current_thd->change_item_tree(args, (*ref0)); > DBUG_PRINT("info", ("actual fix fields")); > > cache->setup(args[0]); > diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc > index 4c11808..e04f5ff 100644 > --- a/sql/item_subselect.cc > +++ b/sql/item_subselect.cc > @@ -1360,7 +1360,7 @@ Item_in_subselect::Item_in_subselect(Item * left_exp, > upper_item(0) > { > DBUG_ENTER("Item_in_subselect::Item_in_subselect"); > - left_expr= left_exp; > + left_expr_orig= left_expr= left_exp; > func= &eq_creator; > init(select_lex, new select_exists_subselect(this)); > max_columns= UINT_MAX; > @@ -1384,7 +1384,7 @@ Item_allany_subselect::Item_allany_subselect(Item * left_exp, > :Item_in_subselect(), func_creator(fc), all(all_arg) > { > DBUG_ENTER("Item_allany_subselect::Item_allany_subselect"); > - left_expr= left_exp; > + left_expr_orig= left_expr= left_exp; > func= func_creator(all_arg); > init(select_lex, new select_exists_subselect(this)); > max_columns= 1; > @@ -2584,15 +2584,13 @@ Item_in_subselect::select_in_like_transformer(JOIN *join) > arena= thd->activate_stmt_arena_if_needed(&backup); > if (!optimizer) > { > - result= (!(optimizer= new Item_in_optimizer(left_expr, this))); > + result= (!(optimizer= new Item_in_optimizer(left_expr_orig, this))); > if (result) > goto out; > } > > thd->lex->current_select= current->return_after_parsing(); > result= optimizer->fix_left(thd, optimizer->arguments()); > - /* fix_fields can change reference to left_expr, we need reassign it */ > - left_expr= optimizer->arguments()[0]; > thd->lex->current_select= current; > > if (changed) > diff --git a/sql/item_subselect.h b/sql/item_subselect.h > index 592e771..930bd66 100644 > --- a/sql/item_subselect.h > +++ b/sql/item_subselect.h > @@ -449,6 +449,7 @@ class Item_in_subselect :public Item_exists_subselect > Item **having_item); > public: > Item *left_expr; > + Item *left_expr_orig; > /* Priority of this predicate in the convert-to-semi-join-nest process. */ > int sj_convert_priority; > /* > diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc > index 5b1a7f2..1363be0 100644 > --- a/sql/opt_subselect.cc > +++ b/sql/opt_subselect.cc > @@ -1593,7 +1593,9 @@ static bool convert_subq_to_sj(JOIN *parent_join, Item_in_subselect *subq_pred) > { > nested_join->sj_outer_expr_list.push_back(subq_pred->left_expr); > Item_func_eq *item_eq= > - new Item_func_eq(subq_pred->left_expr, subq_lex->ref_pointer_array[0]); > + new Item_func_eq(subq_pred->left_expr_orig, subq_lex->ref_pointer_array[0]); > + if (subq_pred->left_expr_orig != subq_pred->left_expr) > + thd->change_item_tree(item_eq->arguments(), subq_pred->left_expr); > item_eq->in_equality_no= 0; > sj_nest->sj_on_expr= and_items(sj_nest->sj_on_expr, item_eq); > } > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits -- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 1

[Maria-developers] [GSoC] MXS-2
by sriram patil 08 Jun '15

08 Jun '15

Hi All, There is no blog post this week. Could not conclude the task at hand. Will be writing a blog post for 2nd and 3rd week together. Thanks, Sriram

1 0

[Maria-developers] 5.5.44 Sprint: MDEV-8067 correct fix for MySQL Bug # 19699237: UNINITIALIZED VARIABLE IN ITEM_FIELD::STR_RESULT
by Alexander Barkov 08 Jun '15

08 Jun '15

Hi Sergei, Please review my patch for MDEV-8067. Some subselect_(mat|sj) test chanted "EXPLAIN" output. Perhaps the new results are correct. Please advise. Thanks.

1 0

[Maria-developers] Please review MDEV-7269 mysqlbinlog Don't know how to handle column type=0 meta=0 (0000)#
by Alexander Barkov 05 Jun '15

05 Jun '15

Hi Sergei, Please review a joint patch for: MDEV-7269 mysqlbinlog Don't know how to handle column type=0 meta=0 (0000)# MDEV-8267 Add /*old*/ comment into I_S.COLUMN_TYPE for old DECIMAL Thanks.

2 4