List:Commits« Previous MessageNext Message »
From:Sven Sandberg Date:September 30 2009 3:54pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)
Bug#46640
View as plain text  
Hi Dao-Gang,

I think the change you describe in your email is not equivalent to what 
the code was before your patch. If, before your patch, 
Start_log_event_v3::do_apply_event(rli) returns non-zero, then 
do_update_pos is not called, so 
rli->relay_log.description_event_for_exec is not updated. After your 
patch, rli->relay_log.description_event_for_exec is updated in any case.

So I think a correct patch would be something like this:

=== modified file 'sql/log_event.cc'
--- sql/log_event.cc	2009-09-10 10:05:53 +0000
+++ sql/log_event.cc	2009-09-30 15:28:32 +0000
@@ -3855,6 +3855,7 @@ bool Format_description_log_event::write
  #if defined(HAVE_REPLICATION) && !defined(MYSQL_CLIENT)
  int Format_description_log_event::do_apply_event(Relay_log_info const 
*rli)
  {
+  int ret= 0;
    DBUG_ENTER("Format_description_log_event::do_apply_event");

  #ifdef USING_TRANSACTIONS
@@ -3896,16 +3897,20 @@ int Format_description_log_event::do_app
        0, then 96, then jump to first really asked event (which is
        >96). So this is ok.
      */
-    DBUG_RETURN(Start_log_event_v3::do_apply_event(rli));
+    ret= Start_log_event_v3::do_apply_event(rli);
    }
-  DBUG_RETURN(0);
+
+  if (!ret) {
+    /* save the information describing this binlog */
+    delete rli->relay_log.description_event_for_exec;
+    const_cast<Relay_log_info 
*>(rli)->relay_log.description_event_for_exec= this;
+  }
+
+  DBUG_RETURN(ret);
  }

  int Format_description_log_event::do_update_pos(Relay_log_info *rli)
  {
-  /* save the information describing this binlog */
-  delete rli->relay_log.description_event_for_exec;
-  rli->relay_log.description_event_for_exec= this;

    if (server_id == (uint32) ::server_id)
    {

===EOF

I tried this with rpl_row_4_bytes and rpl_known_bugs_detection and they 
passed.

/Sven


Daogang Qu wrote:
> Hi Sven,
> If we move code from 'Format_description_log_event::do_update_pos' to
> 'Format_description_log_event::do_apply_event' as following:
> 
>   DBUG_RETURN(Start_log_event_v3::do_apply_event(rli));
>  }
>  /* save the information describing this binlog */
>  delete 
> (const_cast<Relay_log_info*>(rli))->relay_log.description_event_for_exec;
>  
> (const_cast<Relay_log_info*>(rli))->relay_log.description_event_for_exec= 
> this;
> 
>  DBUG_RETURN(0);
> }
> 
> int Format_description_log_event::do_update_pos(Relay_log_info *rli)
> {
>  /* save the information describing this binlog */
>  //delete rli->relay_log.description_event_for_exec;
>  //rli->relay_log.description_event_for_exec= this;
> 
> 
> The following test cases are failed , because they didn't always execute 
> the above two functions one by one.
> 
> rpl.rpl_row_4_bytes 'row'                [ fail ]
>        Test ended at 2009-09-30 22:26:33
> 
> CURRENT_TEST: rpl.rpl_row_4_bytes
> mysqltest: At line 26: failed in 'select 
> master_pos_wait('master-bin.000001', 1636, 300)': 2013: Lost connection 
> to MySQL server during query
> 
> 
> rpl.rpl_known_bugs_detection 'stmt'      [ fail ]
> 
> rpl.rpl_known_bugs_detection 'mix'       [ fail ]
> 
> Best Regards,
> 
> Daogang
> 
> 
> Sven Sandberg wrote:
>> Daogang Qu wrote:
>> [...]
>>>>> === modified file 'sql/sql_binlog.cc'
>>>>> --- a/sql/sql_binlog.cc    2009-01-09 12:49:24 +0000
>>>>> +++ b/sql/sql_binlog.cc    2009-09-28 05:34:02 +0000
>> [...]
>>>>> @@ -209,9 +211,27 @@ void 
>>>>> mysql_client_binlog_statement(THD*          reporting.
>>>>>        */
>>>>>  #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
>>>>> -      if (apply_event_and_update_pos(ev, thd, thd->rli_fake,
> FALSE))
>>>>> +      err= ev->apply_event(rli);
>>>>> +#else
>>>>> +      err= 0;
>>>>> +#endif
>>>>> +      if (ev->get_type_code() == FORMAT_DESCRIPTION_EVENT)
>>>>> +      {
>>>>> +        /*
>>>>> +          Install this as the format description event to use for
> any
>>>>> +          following events.
>>>>> +        */
>>>>> +        delete rli->relay_log.description_event_for_exec;
>>>>> +        rli->relay_log.description_event_for_exec=
>>>>> +          static_cast<Format_description_log_event *>(ev);
>>>>> +      }
>>>>> +      else
>>>>
>>>> (S2)
>>>> This duplicates code from 
>>>> Format_description_log_event::do_update_pos. Can't you move it to 
>>>> Format_description_log_event::do_apply_event instead? I think it 
>>>> makes more sense to have it there anyway.
>>> The compile can't be passed, if we move it to 
>>> Format_description_log_event::do_apply_event.
>>
>> What compiler error do you get? The only thing I can think of that 
>> would go wrong is that rli is const, but you can just cast that:
>>
>> delete 
>> (const_cast<Relay_log_info*>rli)->relay_log.description_event_for_exec;
>>
>> etc. Let me know if there are other problems or if there is anything I 
>> can help with!
>>
>> /Sven
>>
>>>>
>>>>>        {
>>>>>          delete ev;
>>>>> +      }
>>>>> +      ev= 0;
>>>>> +      if (err)
>>>>> +      {
>>>>>          /*
>>>>>            TODO: Maybe a better error message since the BINLOG 
>>>>> statement
>>>>>            now contains several events.
>>>>> @@ -219,17 +239,6 @@ void 
>>>>> mysql_client_binlog_statement(THD*          
>>>>> my_error(ER_UNKNOWN_ERROR, MYF(0), "Error executing BINLOG 
>>>>> statement");
>>>>>          goto end;
>>>>>        }
>>>>> -#endif
>>>>> -
>>>>> -      /*
>>>>> -        Format_description_log_event should not be deleted because
> it
>>>>> -        will be used to read info about the relay log's format; it
>>>>> -        will be deleted when the SQL thread does not need it,
>>>>> -        i.e. when this thread terminates.
>>>>> -      */
>>>>> -      if (ev->get_type_code() != FORMAT_DESCRIPTION_EVENT)
>>>>> -        delete ev;
>>>>> -      ev= 0;
>>>>>      }
>>>>>    }
>>>>>  
>>>>> @@ -238,7 +247,7 @@ void mysql_client_binlog_statement(THD*    
>>>>> my_ok(thd);
>>>>>  
>>>>>  end:
>>>>> -  thd->rli_fake->clear_tables_to_lock();
>>>>> +  rli->clear_tables_to_lock();
>>>>>    my_free(buf, MYF(MY_ALLOW_ZERO_PTR));
>>>>>    DBUG_VOID_RETURN;
>>>>>  }
>>>>>
>>>>>
>>>>>
>>>>>
> ------------------------------------------------------------------------ 
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
> 


-- 
Sven Sandberg, Software Engineer
MySQL AB, www.mysql.com
Thread
bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117) Bug#46640Dao-Gang.Qu28 Sep
  • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)Bug#46640Libing Song28 Sep
    • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117) Bug#46640Kristian Nielsen28 Sep
      • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)Bug#46640Libing Song30 Sep
  • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)Bug#46640Sven Sandberg28 Sep
    • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)Bug#46640Daogang Qu29 Sep
      • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)Bug#46640Sven Sandberg29 Sep
        • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)Bug#46640Daogang Qu29 Sep
          • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)Bug#46640He Zhenxing29 Sep
            • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)Bug#46640Daogang Qu29 Sep
        • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)Bug#46640Daogang Qu29 Sep
        • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)Bug#46640Daogang Qu30 Sep
          • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)Bug#46640Sven Sandberg30 Sep
            • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117)Bug#46640Daogang Qu2 Oct