List:Commits« Previous MessageNext Message »
From:Alfranio Correia Date:December 16 2010 10:12am
Subject:Re: bzr commit into mysql-trunk branch (Dao-Gang.Qu:3208) WL#5493
View as plain text  
Hi Daogang,


On 12/16/2010 08:55 AM, Daogang Qu wrote:
> Hi Alfranio,
> Thanks for your comment. See reply in-line.
> Please review:
> http://lists.mysql.com/commits/127027
>
> Best Regards,
>
> Daogang
>
> 2010-12-11 05:05, Alfranio Correia wrote:
>> Hi Daogang,
>>
>>
>> Great work,
>>
>>
>> STATUS
>> ------
>>
>> Conditionally approved.
>>
>> REQUEST
>> ------
>>
>> 1 - We are not writing directly to the index_file so I think
>> we can replace the init_io_cache by a simple seek.
> We are using init_io_cache for efficiently reading from index file
> instead of writing.

You don't need to call init_io_cache everytime you need to read the
index file. You just need to call it once, after opening the file.

I think you should replace other calls to init_io_cache by a seek,
this would me more efficient as these calls to init_io_cache were
used because after the open there was a init_io_cache with write
properties.

>>> +connection master;
>>> +call mtr.add_suppression("Attempting backtrace");
>>> +call mtr.add_suppression("allocated tablespace *., old maximum was 0");
>>> +call mtr.add_suppression("Error in Log_event::read_log_event()");
>>> +call mtr.add_suppression("Buffered warning: Performance schema
>>> disabled");
>>
>> Please, move to this part of the code to beginning and call
>> syn_slave_with_master.
> I had tried to do that. But the test is blocked.
> And we don't need call syn_slave_with_master,
> because the they will be synced by default.
> in worse case, the slave will not cause these
> error at all in the test.

I will check this next week but I am fine with the current approach.


>>> @@ -1378,20 +1380,15 @@ Log_event* Log_event::read_log_event(con
>>> if (crc_check&&
>>> event_checksum_test((uchar *) buf, event_len, alg))
>>> {
>>> -#ifdef MYSQL_CLIENT
>>> *error= "Event crc check failed! Most likely there is event
>>> corruption.";
>>> +#ifdef MYSQL_CLIENT
>>> if (force_opt)
>>> {
>>> ev= new Unknown_log_event(buf, description_event);
>>> DBUG_RETURN(ev);
>>> }
>>> - else
>>> - DBUG_RETURN(NULL);
>>> -#else
>>> - *error= ER(ER_BINLOG_READ_EVENT_CHECKSUM_FAILURE);
>>> - sql_print_error("%s", ER(ER_BINLOG_READ_EVENT_CHECKSUM_FAILURE));
>>> - DBUG_RETURN(NULL);
>>> #endif
>>> + DBUG_RETURN(NULL);
>>> }
>>>
>>> if (event_type> description_event->number_of_event_types&&
>>>
>>>
>>
>> Can you improve your comments here?
>> I don't understan why these changes are necessary.
>
> OK. I will make the decision after talking about it with Andrei.
> In fact, the original code will cause segment error.
>>

ok.

Cheers.

Thread
bzr commit into mysql-trunk branch (Dao-Gang.Qu:3208) WL#5493Dao-Gang.Qu2 Dec
  • Re: bzr commit into mysql-trunk branch (Dao-Gang.Qu:3208) WL#5493Alfranio Correia10 Dec
    • Re: bzr commit into mysql-trunk branch (Dao-Gang.Qu:3208) WL#5493Daogang Qu16 Dec
      • Re: bzr commit into mysql-trunk branch (Dao-Gang.Qu:3208) WL#5493Alfranio Correia16 Dec
        • Re: bzr commit into mysql-trunk branch (Dao-Gang.Qu:3208) WL#5493Daogang Qu16 Dec
          • Re: bzr commit into mysql-trunk branch (Dao-Gang.Qu:3208) WL#5493Alfranio Correia17 Dec
            • Re: bzr commit into mysql-trunk branch (Dao-Gang.Qu:3208) WL#5493Daogang Qu20 Dec
              • Re: bzr commit into mysql-trunk branch (Dao-Gang.Qu:3208) WL#5493Alfranio Correia20 Dec
                • Re: bzr commit into mysql-trunk branch (Dao-Gang.Qu:3208) WL#5493Daogang Qu21 Dec
    • Re: bzr commit into mysql-trunk branch (Dao-Gang.Qu:3208) WL#5493Daogang Qu16 Dec