2010-12-16 18:12, Alfranio Correia wrote:
> 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.
But for every write and purge to the index file, the index file will be
closed and deleted, and then renamed from crash_safe_index_file.
So we need init_io_cache in the process. No chance for seek.
>
>>>> +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.
Thanks!
>
>
>>>> @@ -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.
Replied in another mail.
Best Regards,
Daogang
>
> Cheers.
>
