On 12/20/2010 10:46 AM, Daogang Qu wrote:
> 2010-12-17 07:01, Alfranio Correia wrote:
>> On 12/16/2010 10:47 AM, Daogang Qu wrote:
>>> 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.
>>>
>>
>>
>> I still don't agree with that.
>> Notice that you open the index with init_io_cache(...READ...) and
>> calling init again is only need if you had openned it with WRITE,
>> otherwise a seek is enough.
> It didn't work. Because we have to end_io_catch(index file)
> before close and delete it. Thanks!
Hi Daogang,
See in what follows what I am suggesting to do. I executed some MTR
tests and everything seems to be ok. I don't know, however, if there
is any corner case. Please, if you agree, double check if there is
anything missing.
Please, check BUG#58953 too and make sure your patch does not
introduce a regression.
Cheers.
=== modified file 'sql/binlog.cc'
--- sql/binlog.cc 2010-12-20 12:59:06 +0000
+++ sql/binlog.cc 2010-12-20 13:06:55 +0000
@@ -1538,7 +1538,7 @@
MYF(MY_WME))) < 0 ||
mysql_file_sync(index_file_nr, MYF(MY_WME)) ||
init_io_cache(&index_file, index_file_nr,
- IO_SIZE, WRITE_CACHE,
+ IO_SIZE, READ_CACHE,
mysql_file_seek(index_file_nr, 0L, MY_SEEK_END,
MYF(0)),
0, MYF(MY_WME | MY_WAIT_IF_FULL)) ||
DBUG_EVALUATE_IF("fault_injection_openning_index", 1, 0))
@@ -2006,7 +2006,7 @@
mysql_mutex_assert_owner(&LOCK_index);
/* As the file is flushed, we can't get an error here */
- (void) reinit_io_cache(&index_file, READ_CACHE, (my_off_t) 0, 0, 0);
+ my_b_seek(&index_file, (my_off_t) 0);
for (;;)
{
@@ -2077,8 +2077,7 @@
mysql_mutex_assert_owner(&LOCK_index);
/* As the file is flushed, we can't get an error here */
- (void) reinit_io_cache(&index_file, READ_CACHE,
linfo->index_file_offset, 0,
- 0);
+ my_b_seek(&index_file, linfo->index_file_offset);
linfo->index_file_start_offset= linfo->index_file_offset;
if ((length=my_b_gets(&index_file, fname, FN_REFLEN)) <= 1)
>
> Best Regards,
>
> Daogang
>>
>> I will check this next week.
>>
>> Cheers.
>