Re: bzr commit into mysql-5.0-bugteam branch (alfranio.correia:2704) Bug#31665 Bug#35542 Bug#40337

[alfranio correia]

He Zhenxing wrote:
> alfranio correia wrote:
>   
>> Hi,
>>
>>
>> In-line comments.
>>
>> He Zhenxing wrote:
>>     
>>> alfranio correia wrote: 
>>>   
>>>       
>>>> Alfranio Correia wrote:
>>>>     
>>>>         
>>>>> #At
> file:///home/acorreia/workspace.sun/playground.mysql/mysql-5.0.68-custom_build/
>>>>>
>>>>>  2704 Alfranio Correia	2008-11-07
>>>>>       Added option to sync master.info (--sync-master-info, integer)
> 
>>>>>       and relay log (--sync-relay-log, integer) to disk after 
>>>>>       every event and the relay info (--sync-relay-log-info, boolean)
> per transaction. 
>>>>>       This supersedes the patch proposed 
>>>>>       by Sinisa and modified by He Zhenxing to fix bugs: BUG#35542
> and BUG#31665. This is an update to BUG#40337.
>>>>> modified:
>>>>>   sql/log.cc
>>>>>   sql/mysql_priv.h
>>>>>   sql/mysqld.cc
>>>>>   sql/set_var.cc
>>>>>   sql/set_var.h
>>>>>   sql/slave.cc
>>>>>   sql/sql_class.h
>>>>>
>>>>> === modified file 'sql/log.cc'
>>>>> --- a/sql/log.cc	2008-07-24 12:28:21 +0000
>>>>> +++ b/sql/log.cc	2008-11-07 19:21:48 +0000
>>>>> @@ -32,8 +32,7 @@
>>>>>  #include "message.h"
>>>>>  #endif
>>>>>  
>>>>> -MYSQL_LOG mysql_log, mysql_slow_log, mysql_bin_log;
>>>>> -ulong sync_binlog_counter= 0;
>>>>> +MYSQL_LOG mysql_log, mysql_slow_log,
> mysql_bin_log(&sync_binlog_period);
>>>>>  
>>>>>  static Muted_query_log_event invisible_commit;
>>>>>  
>>>>> @@ -402,10 +401,11 @@ static int find_uniq_filename(char *name
>>>>>  }
>>>>>  
>>>>>  
>>>>> -MYSQL_LOG::MYSQL_LOG()
>>>>> +MYSQL_LOG::MYSQL_LOG(uint *sync_period)
>>>>>    :bytes_written(0), last_time(0), query_start(0), name(0),
>>>>>     prepared_xids(0), log_type(LOG_CLOSED), file_id(1),
> open_count(1),
>>>>>     write_error(FALSE), inited(FALSE), need_start_event(TRUE),
>>>>> +   sync_period_ptr(sync_period),
>>>>>     description_event_for_exec(0), description_event_for_queue(0)
>>>>>  {
>>>>>    /*
>>>>> @@ -1622,6 +1622,8 @@ bool MYSQL_LOG::append(Log_event* ev)
>>>>>    }
>>>>>    bytes_written+= ev->data_written;
>>>>>    DBUG_PRINT("info",("max_size: %lu",max_size));
>>>>> +  if (flush_and_sync())
>>>>> +    goto err;
>>>>>    if ((uint) my_b_append_tell(&log_file) > max_size)
>>>>>      new_file(0);
>>>>>  
>>>>> @@ -1652,6 +1654,8 @@ bool MYSQL_LOG::appendv(const char* buf,
>>>>>      bytes_written += len;
>>>>>    } while ((buf=va_arg(args,const char*)) &&
> (len=va_arg(args,uint)));
>>>>>    DBUG_PRINT("info",("max_size: %lu",max_size));
>>>>> +  if (flush_and_sync())
>>>>> +    goto err;
>>>>>    if ((uint) my_b_append_tell(&log_file) > max_size)
>>>>>      new_file(0);
>>>>>  
>>>>> @@ -1758,9 +1762,10 @@ bool MYSQL_LOG::flush_and_sync()
>>>>>    safe_mutex_assert_owner(&LOCK_log);
>>>>>    if (flush_io_cache(&log_file))
>>>>>      return 1;
>>>>> -  if (++sync_binlog_counter >= sync_binlog_period &&
> sync_binlog_period)
>>>>> +  uint sync_period= get_sync_period();
>>>>> +  if (sync_period && ++sync_counter >= sync_period)
>>>>>    {
>>>>> -    sync_binlog_counter= 0;
>>>>> +    sync_counter= 0;
>>>>>      err=my_sync(fd, MYF(MY_WME));
>>>>>    }
>>>>>    return err;
>>>>>
>>>>> === modified file 'sql/mysql_priv.h'
>>>>> --- a/sql/mysql_priv.h	2008-10-02 11:57:52 +0000
>>>>> +++ b/sql/mysql_priv.h	2008-11-07 19:21:48 +0000
>>>>> @@ -1310,7 +1310,8 @@ extern ulong max_binlog_size, max_relay_
>>>>>  extern ulong rpl_recovery_rank, thread_cache_size;
>>>>>  extern ulong back_log;
>>>>>  extern ulong specialflag, current_pid;
>>>>> -extern ulong expire_logs_days, sync_binlog_period,
> sync_binlog_counter;
>>>>> +extern ulong expire_logs_days;
>>>>> +extern uint sync_binlog_period, sync_relaylog_period,
> sync_relayloginfo_period, sync_masterinfo_period;
>>>>>   
>>>>>       
>>>>>           
>>>> Jasonh, any reason to change from ulong to uint?
>>>>
>>>>     
>>>>         
>>> read and write int are supposed to be atomic on all platforms supported
>>> by MySQL, so we don't need to add LOCKs to protect the access of these
>>> variables. And we cannot expect the same for long.
>>>
>>>   
>>>       
>> Do we really need this?
>> I think we can survive without the atomicity in this case.
>>
>>     
>
> these variables can be changed by user at runtime by using SET command,
> so it is possible that two threads are reading and writing the value of
> the variable at the same time.
>
> [snip]
>   
My point is that we don't need any mutex, atomic variables, etc, etc.
You are not updating a structure that will be corrupted due to a race
condition.
Anyway, I will keep the integer value.