On 11/17/2010 01:29 AM, Daogang Qu wrote:
> 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:
>>
>> /*
>> 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."
> Should we upgrade T_WRITE_DELAYED lock to T_WRITE in mixed mode if we
> remove above lines?
> I we don't do it. The INSERT DELAYED stmt still will be executed by
> DELAYED thread in SBR by
> default.
I think we should keep *one* call to
set_current_stmt_binlog_format_row_if_mixed(), remove the other call and
remove the calls to set_stmt_unsafe(). Then we can use T_WRITE_DELAYED.
/Sven
>
> Best Regards,
>
> Daogang
>>
>> /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<binlog_start>;
>>>
>>> === 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<binlog_start>;
>>> @@ -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<binlog_start>;
>>>
>>> === 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);
>>>
>>>
>>>
>>>
>>>
>>
>>
>
