List:Commits« Previous MessageNext Message »
From:Alfranio Correia Date:December 20 2010 1:22pm
Subject:Re: bzr commit into mysql-trunk branch (Dao-Gang.Qu:3208) WL#5493
View as plain text  
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.
>

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