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

[He Zhenxing]

alfranio correia wrote:
> 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.


if the write to an integer is not atomic, e.g. it will first update the
lower bits and then the higher bits(or the other way).

Consider this senario, on 32bit machine, and long is 64bit, th1 and th2
want to update the long value to 0x1 and 0x100000000 respectively:
th1: udpate the lower 32 bits to 0x00000001
th2: update the lower 32 bits to 0x00000000
th2: udpate the higer 32 bits to 0x00000001
th1: update the higer 32 bits to 0x00000000

So after both complete, the value will be 0

> Anyway, I will keep the integer value.

OK