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

[He Zhenxing]

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]