Sergei, hello.
Thanks for looking!
> Hi!
>
> On Oct 16, Andrei Elkin wrote:
>> ChangeSet@stripped, 2007-10-16 19:19:57+03:00,
> aelkin@stripped +5 -0
>> Bug #27571 asynchronousity in setting mysql_`query`::error and
> Query_log_event::error_code
>>
>> A query can perform completely having the local var error of
>> mysql_$query zero, where $query in insert, update, delete, load, and
>> be binlogged with error_code e.g KILLED_QUERY while there is no
>> reason do to so. That can happen because Query_log_event consults
>> thd->killed flag to evaluate error_code.
>>
>> Fixed with implementing a scheme suggested and partly implemented at
>> time of bug@22725 work-on. error_status is cached immediatly after
>> the control leaves the main rows-loop and that instance always
>> corresponds to `error' the local of mysql_$query functions. The
>> cached value is passed to Query_log_event constructor, not the
>> default thd->killed which can be changed in between of the caching
>> and the constructing.
>>
>> A simulation test is provided although it can not be easily made
>> deterministic. So it has to be moved to the manual suite.
>>
>> The current changeset is limited to capture only UPDATE query. The
>> rest of modifying DMS:s will be handled by deploying the current
>> pattern upon successful review of it.
>>
>> --- a/mysql-test/t/binlog_killed.test 2007-06-07 21:25:21 +03:00
>> +++ b/mysql-test/t/binlog_killed.test 2007-10-16 19:19:54 +03:00
>> @@ -32,6 +32,7 @@ disable_abort_on_error;
>> disable_query_log;
>> disable_result_log;
>>
>> +--replace_result $ID ID
>
> what for ?
> (also applies to all changes below)
>
It is intended for the test to be determinsic. Besides, we don't need the
number corrsponding to the connection.
>> eval kill query $ID;
>>
>> @@ -244,5 +251,91 @@ drop function bug27565;
>>
>> system rm $MYSQLTEST_VARDIR/tmp/kill_query_calling_sp.binlog ;
>>
>> +#
>> +# bug#27571 asynchronous setting mysql_`query`::error and
> Query_log_e::error_code
>> +#
>> +
>> +# checking that killing inside of select loops is safe as before
>> +# killing after the loop can be only simulated - another test
>
> But the first test case in binlog_killed.test already checks that,
> doesn't it ?
>
>> diff -Nrup a/sql/sql_update.cc b/sql/sql_update.cc
>> --- a/sql/sql_update.cc 2007-10-13 15:49:39 +03:00
>> +++ b/sql/sql_update.cc 2007-10-16 19:19:54 +03:00
>> @@ -134,6 +134,7 @@ int mysql_update(THD *thd,
>> SELECT_LEX *select_lex= &thd->lex->select_lex;
>> bool need_reopen;
>> List<Item> all_fields;
>> + THD::killed_state killed_status= THD::NOT_KILLED;
>> DBUG_ENTER("mysql_update");
>>
>> LINT_INIT(timestamp_query_id);
>> @@ -519,43 +520,22 @@ int mysql_update(THD *thd,
>> table->file->unlock_row();
>> thd->row_count++;
>> }
>> + /*
>> + caching the killed status to pass as the arg to query event constuctor;
>> + the status not the cached value can change since this point
>> + still w/o any harm for binlogging.
>> + It's assumed that if an error was set in combination with an effective
>> + killed status then the error is due to killing.
>
> sorry, I cannot parse the above comment, try to reformulate it.
Agree, will do.
Much simpler would be to say that
the cached value can not change whereas the killed status can
(externally) since this point and change of the latter won't affect
binlogging.
>
>> + */
>> + killed_status= thd->killed; // get the status of the volatile
>
> The rest is ok
>
> Regards / Mit vielen Grüssen,
> Sergei
>
> --
> __ ___ ___ ____ __
> / |/ /_ __/ __/ __ \/ / Sergei Golubchik <serg@stripped>
> / /|_/ / // /\ \/ /_/ / /__ Principal Software Developer
> /_/ /_/\_, /___/\___\_\___/ MySQL GmbH, Dachauer Str. 37, D-80335 München
> <___/ Geschäftsführer: Kaj Arnö - HRB
> München 162140
I am expanding this changeset to cover other query types.
regards,
Andrei
