List:Internals« Previous MessageNext Message »
From:Tianwei Date:March 23 2010 2:36pm
Subject:Re: Is that intentional or my configure error?
View as plain text  
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.
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