From: Daogang Qu Date: November 15 2010 11:13pm Subject: Re: bzr commit into mysql-5.5-bugteam branch (Dao-Gang.Qu:3118) Bug#57666 List-Archive: http://lists.mysql.com/commits/123959 Message-Id: <4CE1BEB7.2080803@sun.com> MIME-Version: 1.0 Content-Type: text/plain; CHARSET=US-ASCII; format=flowed Content-Transfer-Encoding: 7BIT 2010-11-15 22:26, Sven Sandberg wrote: > Hi Daogang, > > The patch looks good, I approve it. > > I would suggest one more simplification though. Fix it if you like, > but you don't have to. In Delayed_insert::Delayed_insert() there is: Thanks for the suggestion, I will fix it after check. Thanks! Best Regards, Daogang > > /* > Statement-based replication of INSERT DELAYED has problems with > RAND() and user variables, so in mixed mode we go to row-based. > For normal commands, the unsafe flag is set at parse time. > However, since the flag is a member of the THD object, of which > the delayed_insert thread has its own copy, we must set the > statement to unsafe here and explicitly set row logging mode. > > @todo set_current_stmt_binlog_format_row_if_mixed should not be > called by anything else than thd->decide_logging_format(). When > we call set_current_blah here, none of the checks in > decide_logging_format is made. We should probably call > thd->decide_logging_format() directly instead. /Sven > */ > thd.lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_DELAYED); > thd.set_current_stmt_binlog_format_row_if_mixed(); > > In handle_delayed_inserts there is: > > /* > Statement-based replication of INSERT DELAYED has problems with > RAND() > and user vars, so in mixed mode we go to row-based. > */ > thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_DELAYED); > thd->set_current_stmt_binlog_format_row_if_mixed(); > > > It is unnecessary to do this in two places, so I think we can remove > both lines from the constructor. Also, it is not necessary to call > set_stm_unsafe() at all since we call > set_current_stmt_format_row_if_mixed(). set_stm_unsafe() is only > meaningful if there is a subsequent call to decide_logging_format(), > which is not the case for the insert delayed thread. Also, the > comments are misleading. I think a better comment would be something > like "INSERT DELAYED has to be logged in row mode because the time at > which rows are inserted cannot be determined." > > /Sven > > > On 11/11/2010 10:49 AM, Dao-Gang.Qu@stripped wrote: >> #At file:///home/daogang/bzrwork/bug57666/mysql-5.5-bugteam/ based on >> revid:vvaintroub@stripped >> >> 3118 Dao-Gang.Qu@stripped 2010-11-11 >> Bug #57666 Unclear warning with broken text in error log on >> INSERT DELAYED >> >> It is not necessary to support INSERT DELAYED for a single >> value insert, >> while we do not support that for multi-values insert when >> binlog is >> enabled in SBR. >> >> The lock_type is upgrade to TL_WRITE from TL_WRITE_DELAYED for >> INSERT DELAYED for single value insert as multi-values insert >> did when binlog is enabled. Then it's safe. And binlog it as >> INSERT without DELAYED. >> @ mysql-test/extra/binlog_tests/binlog_insert_delayed.test >> Added test case for bug#57666. >> @ mysql-test/suite/binlog/r/binlog_row_binlog.result >> Updated for bug#57666 >> @ mysql-test/suite/binlog/r/binlog_statement_insert_delayed.result >> Updated for bug#57666 >> @ mysql-test/suite/binlog/r/binlog_stm_binlog.result >> Updated for bug#57666 >> @ sql/sql_insert.cc >> Updated code for the following things: >> 1. Upgrade the lock_type to TL_WRITE from TL_WRITE_DELAYED >> for INSERT DELAYED for single value insert as multi-values >> insert did when binlog is enabled. >> 2. Clear code to not binlog INSERT DELAYED STMT in SBR. >> 3. Get rid of privilege check for log_on. >> >> modified: >> mysql-test/extra/binlog_tests/binlog_insert_delayed.test >> mysql-test/suite/binlog/r/binlog_row_binlog.result >> mysql-test/suite/binlog/r/binlog_statement_insert_delayed.result >> mysql-test/suite/binlog/r/binlog_stm_binlog.result >> sql/sql_insert.cc >> === modified file >> 'mysql-test/extra/binlog_tests/binlog_insert_delayed.test' >> --- a/mysql-test/extra/binlog_tests/binlog_insert_delayed.test >> 2010-08-30 06:03:28 +0000 >> +++ b/mysql-test/extra/binlog_tests/binlog_insert_delayed.test >> 2010-11-11 09:49:28 +0000 >> @@ -34,11 +34,11 @@ create table t1 (a int not null auto_inc >> let $table=t1; >> let $count=0; >> >> -insert delayed into t1 values (207); >> +insert /* before delayed */ delayed /* after delayed */ into t1 >> values (207); >> inc $count; >> --source include/wait_until_rows_count.inc >> >> -insert delayed into t1 values (null); >> +insert /*! delayed */ into t1 values (null); >> inc $count; >> --source include/wait_until_rows_count.inc >> >> >> === modified file 'mysql-test/suite/binlog/r/binlog_row_binlog.result' >> --- a/mysql-test/suite/binlog/r/binlog_row_binlog.result >> 2010-08-30 06:03:28 +0000 >> +++ b/mysql-test/suite/binlog/r/binlog_row_binlog.result >> 2010-11-11 09:49:28 +0000 >> @@ -1218,8 +1218,8 @@ master-bin.000001 # Delete_rows # >> # tabl >> master-bin.000001 # Query # # COMMIT >> drop table t1,t2,t3,tt1; >> create table t1 (a int not null auto_increment, primary key (a)) >> engine=myisam; >> -insert delayed into t1 values (207); >> -insert delayed into t1 values (null); >> +insert /* before delayed */ delayed /* after delayed */ into t1 >> values (207); >> +insert /*! delayed */ into t1 values (null); >> insert delayed into t1 values (300); >> FLUSH TABLES; >> show binlog events from; >> >> === modified file >> 'mysql-test/suite/binlog/r/binlog_statement_insert_delayed.result' >> --- >> a/mysql-test/suite/binlog/r/binlog_statement_insert_delayed.result >> 2010-08-30 06:03:28 +0000 >> +++ >> b/mysql-test/suite/binlog/r/binlog_statement_insert_delayed.result >> 2010-11-11 09:49:28 +0000 >> @@ -1,6 +1,6 @@ >> create table t1 (a int not null auto_increment, primary key (a)) >> engine=myisam; >> -insert delayed into t1 values (207); >> -insert delayed into t1 values (null); >> +insert /* before delayed */ delayed /* after delayed */ into t1 >> values (207); >> +insert /*! delayed */ into t1 values (null); >> insert delayed into t1 values (300); >> FLUSH TABLES; >> show binlog events from; >> @@ -10,14 +10,14 @@ master-bin.000001 # Query # # use >> `mtr`; >> master-bin.000001 # Query # # COMMIT >> master-bin.000001 # Query # # use `test`; create >> table t1 (a int not null auto_increment, primary key (a)) engine=myisam >> master-bin.000001 # Query # # BEGIN >> -master-bin.000001 # Query # # use `test`; insert >> delayed into t1 values (207) >> +master-bin.000001 # Query # # use `test`; insert /* >> before delayed */ /* after delayed */ into t1 values (207) >> master-bin.000001 # Query # # COMMIT >> master-bin.000001 # Query # # BEGIN >> master-bin.000001 # Intvar # # INSERT_ID=208 >> -master-bin.000001 # Query # # use `test`; insert >> delayed into t1 values (null) >> +master-bin.000001 # Query # # use `test`; insert /*! >> */ into t1 values (null) >> master-bin.000001 # Query # # COMMIT >> master-bin.000001 # Query # # BEGIN >> -master-bin.000001 # Query # # use `test`; insert >> delayed into t1 values (300) >> +master-bin.000001 # Query # # use `test`; insert >> into t1 values (300) >> master-bin.000001 # Query # # COMMIT >> master-bin.000001 # Query # # use `test`; FLUSH TABLES >> RESET MASTER; >> >> === modified file 'mysql-test/suite/binlog/r/binlog_stm_binlog.result' >> --- a/mysql-test/suite/binlog/r/binlog_stm_binlog.result >> 2010-08-30 06:03:28 +0000 >> +++ b/mysql-test/suite/binlog/r/binlog_stm_binlog.result >> 2010-11-11 09:49:28 +0000 >> @@ -717,8 +717,8 @@ master-bin.000001 # Query # # use >> `mysql >> master-bin.000001 # Query # # COMMIT >> drop table t1,t2,t3,tt1; >> create table t1 (a int not null auto_increment, primary key (a)) >> engine=myisam; >> -insert delayed into t1 values (207); >> -insert delayed into t1 values (null); >> +insert /* before delayed */ delayed /* after delayed */ into t1 >> values (207); >> +insert /*! delayed */ into t1 values (null); >> insert delayed into t1 values (300); >> FLUSH TABLES; >> show binlog events from; >> >> === modified file 'sql/sql_insert.cc' >> --- a/sql/sql_insert.cc 2010-10-07 10:01:51 +0000 >> +++ b/sql/sql_insert.cc 2010-11-11 09:49:28 +0000 >> @@ -419,8 +419,7 @@ void prepare_triggers_for_insert_stmt(TA >> >> static >> void upgrade_lock_type(THD *thd, thr_lock_type *lock_type, >> - enum_duplicates duplic, >> - bool is_multi_insert) >> + enum_duplicates duplic) >> { >> if (duplic == DUP_UPDATE || >> (duplic == DUP_REPLACE&& *lock_type == >> TL_WRITE_CONCURRENT_INSERT)) >> @@ -469,10 +468,9 @@ void upgrade_lock_type(THD *thd, thr_loc >> return; >> } >> >> - bool log_on= (thd->variables.option_bits& OPTION_BIN_LOG || >> - ! (thd->security_ctx->master_access& SUPER_ACL)); >> + bool log_on= (thd->variables.option_bits& OPTION_BIN_LOG); >> if (global_system_variables.binlog_format == BINLOG_FORMAT_STMT&& >> - log_on&& mysql_bin_log.is_open()&& is_multi_insert) >> + log_on&& mysql_bin_log.is_open()) >> { >> /* >> Statement-based binary logging does not work in this case, >> because: >> @@ -680,8 +678,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t >> By default, both logs are enabled (this won't cause problems if >> the server >> runs without --log-bin). >> */ >> - bool log_on= ((thd->variables.option_bits& OPTION_BIN_LOG) || >> - (!(thd->security_ctx->master_access& SUPER_ACL))); >> + bool log_on= (thd->variables.option_bits& OPTION_BIN_LOG); >> #endif >> thr_lock_type lock_type; >> Item *unused_conds= 0; >> @@ -691,8 +688,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t >> Upgrade lock type if the requested lock is incompatible with >> the current connection mode or table operation. >> */ >> - upgrade_lock_type(thd,&table_list->lock_type, duplic, >> - values_list.elements> 1); >> + upgrade_lock_type(thd,&table_list->lock_type, duplic); >> >> /* >> We can't write-delayed into a table locked with LOCK TABLES: >> @@ -1025,7 +1021,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t >> DBUG_ASSERT(thd->killed != THD::KILL_BAD_DATA || error> 0); >> if (was_insert_delayed&& table_list->lock_type == TL_WRITE) >> { >> - /* Binlog multi INSERT DELAYED as INSERT without DELAYED. */ >> + /* Binlog INSERT DELAYED as INSERT without DELAYED. */ >> String log_query; >> if (create_insert_stmt_from_insert_delayed(thd,&log_query)) >> { >> @@ -2912,6 +2908,13 @@ bool Delayed_insert::handle_inserts(void >> if (log_query) >> { >> /* >> + Guaranteed that the INSERT DELAYED STMT will not be here >> + in SBR when mysql binlog is enabled. >> + */ >> + DBUG_ASSERT(!(mysql_bin_log.is_open()&& >> + !thd.is_current_stmt_binlog_format_row())); >> + >> + /* >> This is the first value of an INSERT statement. >> It is the right place to clear a forced insert_id. >> This is usually done after the last value of an INSERT >> statement, >> @@ -2978,39 +2981,6 @@ bool Delayed_insert::handle_inserts(void >> table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); >> } >> >> - if (log_query&& mysql_bin_log.is_open()) >> - { >> - bool backup_time_zone_used = thd.time_zone_used; >> - Time_zone *backup_time_zone = thd.variables.time_zone; >> - if (row->time_zone != NULL) >> - { >> - thd.time_zone_used = true; >> - thd.variables.time_zone = row->time_zone; >> - } >> - >> - /* if the delayed insert was killed, the killed status is >> - ignored while binlogging */ >> - int errcode= 0; >> - if (thd.killed == THD::NOT_KILLED) >> - errcode= query_error_code(&thd, TRUE); >> - >> - /* >> - If the query has several rows to insert, only the first row >> will come >> - here. In row-based binlogging, this means that the first row >> will be >> - written to binlog as one Table_map event and one Rows event >> (due to an >> - event flush done in binlog_query()), then all other rows of >> this query >> - will be binlogged together as one single Table_map event and >> one >> - single Rows event. >> - */ >> - if (thd.binlog_query(THD::ROW_QUERY_TYPE, >> - row->query.str, row->query.length, >> - FALSE, FALSE, FALSE, errcode)) >> - goto err; >> - >> - thd.time_zone_used = backup_time_zone_used; >> - thd.variables.time_zone = backup_time_zone; >> - } >> - >> if (table->s->blob_fields) >> free_delayed_insert_blobs(table); >> thread_safe_decrement(delayed_rows_in_use,&LOCK_delayed_status); >> >> >> >> >> > >