List:Internals« Previous MessageNext Message »
From:Sergey Vojtovich Date:March 23 2010 6:39pm
Subject:Re: Is that intentional or my configure error?
View as plain text  
Afaik the only lock that guarantees consistent shared MyISAM status
(which includes data_file_length) is MYISAM_SHARE::lock::mutex.

Frankly speaking I do not fully understand this code, but here must
be a race condition.

On Tue, Mar 23, 2010 at 10:36:23PM +0800, Tianwei wrote:
> BTW, could anyone also kindly help me look at the following problem, I
> personally talked this with Kristofer When I learn the bug
> http://bugs.mysql.com/bug.php?id=28249,
> during the study, I can not quite understand how the following work.
> This is still from 5.0.
> ===>
> One problem I want to learn how two different threads update the the
> state.data_file_length.
> the following is my experiment setting up:
>   a. one thread issue "select"
>   b. the other thread issue "concurrent insert"
> 
> 1. for "select", first check the way it used in register_query_cache_table
>     my_bool ha_myisam::register_query_cache_table(...) {
>         actual_data_file_length= file->s->state.state.data_file_length;
>     }
> 
>   where this function is recursively called by the following call-chain:
>      Query_cache::store_query() {
>           STRUCT_LOCK(&structure_guard_mutex);
>           ask_handler_allowance(thd, tables_used)
>           STRUCT_UNLOCK(&structure_guard_mutex);
>      }
>    my_bool Query_cache::ask_handler_allowance(THD *thd,
>                        TABLE_LIST *tables_used)
>   {
>     if (!handler->register_query_cache_table(thd,
> table->s->table_cache_key,
>                          table->s->key_length,
>                          &tables_used->callback_func,
>                          &tables_used->engine_data))
> 
>   }
> 
> so we knew that the read of "state.data_file_length" is protected by
> structure_guard_mutex.
> 
> 2. insert
> The way we check the insert is that:
> (gdb) down
> #0  mysql_insert (thd=0x7ffff0005380, table_list=0xdb3be0, fields=...,
> values_list=..., update_fields=..., update_values=...,\
>  duplic=DUP_ERROR, ignore=false) at sql_insert.cc:630
> (gdb) p
> &(table_list->table->file->file->s->state.state.data_file_length)
> $13 = (my_off_t *) 0x7ffff002b2d0
> (gdb) p *(unsigned long)0x7ffff002b2d0
> $14 = 21
> (gdb) watch *(unsigned long)0x7ffff002b2d0
> Hardware watchpoint 5: *(unsigned long)0x7ffff002b2d0
> 
> it will stop at:
> Old value = 21
> New value = 28
> 0x0000000000877b1d in mi_update_status (param=0xd695a0) at mi_locking.c:308
> Current language:  auto
> 
> the stack trace:
> (gdb) bt
> #0  0x0000000000877b1d in mi_update_status (param=0xd695a0) at mi_locking.c:308
> #1  0x00000000008f0f67 in thr_unlock (data=0xd698a8) at thr_lock.c:771
> #2  0x00000000008f1945 in thr_multi_unlock (data=0xd701c8, count=1) at
> thr_lock.c:1029
> #3  0x00000000005b0845 in mysql_unlock_tables (thd=0x7ffff0005380,
> sql_lock=0xd701b0) at lock.cc:276
> #4  0x000000000065381b in mysql_insert (thd=0x7ffff0005380,
> table_list=0xdb3be0, fields=..., values_list=..., update_fields=.\
> .., update_values=..., duplic=DUP_ERROR, ignore=false) at sql_insert.cc:939
> #5  0x00000000005d7245 in mysql_execute_command (thd=0x7ffff0005380)
> at sql_parse.cc:3840
> 
> after checking the stack, I found that the same
> state->data_file_length is only protected by the following statement:
> void thr_unlock(THR_LOCK_DATA *data)
> {
>   pthread_mutex_lock(&lock->mutex);
>    (*lock->update_status)(data->status_param);
>  pthread_mutex_unlock(&lock->mutex);
> }
> 
> void mi_update_status(void* param)
> {
>  info->s->state.state= *info->state;
> }
> 
> we can see that the "state->data_file_length" is only protected by
> lock->mutex, not the global structure_guard_mutex.
> 
> 3. if the above is true, then how we can understand the following
> comments as it assumes that data_file_length is protected by the same
> strucutre_guard_mutex: sql/ha_myisam.cc
> +  /*
> +    POSIX visibility rules specify that "2. Whatever memory values a
> +    thread can see when it unlocks a mutex <...> can also be seen by any
> +    thread that later locks the same mutex". In this particular case,
> +    concurrent insert thread had modified the data_file_length in
> +    MYISAM_SHARE before it has unlocked (or even locked)
> +    structure_guard_mutex. So, here we're guaranteed to see at least that
> +    value after we've locked the same mutex. We can see a later value
> +    (modified by some other thread) though, but it's ok, as we only want
> +    to know if the variable was changed, the actual new value doesn't matter
> +  */
> 
> Thanks.
> 
> Tianwei
> 
> On Tue, Mar 23, 2010 at 10:19 PM, Tianwei <tianwei.sheng@stripped> wrote:
> > Hi, Sergey,
> >  Thanks. I guess there should be such nice comment ;-).
> >
> > Tianwei
> > On Tue, Mar 23, 2010 at 10:15 PM, Sergey Vojtovich <svoj@stripped> wrote:
> >> Hi Tianwei,
> >>
> >> in 5.1 (include/my_pthread.h) there is a nice comment regarding
> >> statistic_* functions:
> >>
> >>  statistics_xxx functions are for non critical statistic,
> >>  maintained in global variables.
> >>  When compiling with SAFE_STATISTICS:
> >>  - race conditions can not occur.
> >>  - some locking occurs, which may cause performance degradation.
> >>
> >>  When compiling without SAFE_STATISTICS:
> >>  - race conditions can occur, making the result slightly inaccurate.
> >>  - the lock given is not honored.
> >>
> >> Regards,
> >> Sergey
> >>
> >> On Tue, Mar 23, 2010 at 09:41:41PM +0800, Tianwei wrote:
> >>> Hi, all,
> >>>   I use the following configuration for mysql 5.0:
> >>> CFLAGS="-O0 -g3" CC=gcc CXX=g++ CXXFLAGS="-O0  -g3
> >>> -felide-constructors   -fno-exceptions -fno-rtti" ./configure
> >>> --prefix=/home/tianwei/mysql-server/bin/ --enable-assembler
> >>> I develop a tool which can detect lock usage, It gives the warning as:
> >>>
> >>> Mem: 0x32bf7c0 current lockset is: () previous lockset is: ()
> >>> the thread id are: 7984 8140
> >>> the ip and source files are: 0x006ada00
> >>> /home/tianwei/mysql-server/5.0/sql/ha_myisam.cc:1605
> >>> 0x006ad9f5 /home/tianwei/mysql-server/5.0/sql/ha_myisam.cc:1605
> >>> the stack trace are:
> >>> previous call stack is:
> >>> current call stack is:
> >>>
> >>> I check ha_myisam.cc, it looks like:
> >>> 1604  
> statistic_increment(table->in_use->status_var.ha_read_next_count,
> >>> 1605                  
>     &LOCK_status);
> >>>
> >>> the assembly code for this is:
> >>>   6ad9f5:       48 8b 90 d0 0a 00 00  
>  mov    0xad0(%rax),%rdx
> >>>   6ad9fc:       48 83 c2 01    
>         add    $0x1,%rdx
> >>>   6ada00:       48 89 90 d0 0a 00 00  
>  mov    %rdx,0xad0(%rax)
> >>>
> >>> I check the .ii file which seems that the statistic actually do not
> >>> use lock or atomic_inc,
> >>> int ha_myisam::index_next(byte * buf)
> >>> {
> >>>   ;
> >>>   (table->in_use->status_var.ha_read_next_count)++;
> >>>   int error=mi_rnext(file,buf,active_index);
> >>>   table->status=error ? 2: 0;
> >>>   return error;
> >>> }
> >>>
> >>> I am new for mysqld, I think there should be some configuration flag
> >>> which can enable this, such as not define "HAVE_ATOMIC_ADD" to enable
> >>> pthread lock/unlock pair, or We just tolerate the race here because
> of
> >>> the performance issue and we are OK because it's only statistic as the
> >>> function name indicated?
> >>> Can you give me some suggestions? I build mysqld with gcc/g++ 4.4.1
> on
> >>> Ubuntu 9.10(X86_64)
> >>>
> >>> Thanks so much.
> >>>
> >>> Tianwei
> >>> --
> >>> Sheng, Tianwei
> >>> Inst. of High Performance Computing
> >>> Dept. of Computer Sci. & Tech.
> >>> Tsinghua Univ.
> >>>
> >>> --
> >>> MySQL Internals Mailing List
> >>> For list archives: http://lists.mysql.com/internals
> >>> To unsubscribe:    http://lists.mysql.com/internals?unsub=1
> >>>
> >>
> >> --
> >> Sergey Vojtovich <svoj@stripped>
> >> MySQL AB, Software Engineer
> >> Izhevsk, Russia, www.mysql.com
> >>
> >
> >
> >
> > --
> > Sheng, Tianwei
> > Inst. of High Performance Computing
> > Dept. of Computer Sci. & Tech.
> > Tsinghua Univ.
> >
> 
> 
> 
> -- 
> Sheng, Tianwei
> Inst. of High Performance Computing
> Dept. of Computer Sci. & Tech.
> Tsinghua Univ.
> 
> --
> MySQL Internals Mailing List
> For list archives: http://lists.mysql.com/internals
> To unsubscribe:    http://lists.mysql.com/internals?unsub=1
> 

-- 
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com
Thread
Is that intentional or my configure error?Tianwei23 Mar
  • Re: Is that intentional or my configure error?xiaobing jiang23 Mar
    • Re: Is that intentional or my configure error?Tianwei23 Mar
  • Re: Is that intentional or my configure error?Sergey Vojtovich23 Mar
    • Re: Is that intentional or my configure error?Tianwei23 Mar
      • Re: Is that intentional or my configure error?Tianwei23 Mar
        • Re: Is that intentional or my configure error?Sergey Vojtovich23 Mar
        • Re: Is that intentional or my configure error?Konstantin Osipov23 Mar
Re: Is that intentional or my configure error?Tianwei24 Mar