Re: bk commit into 5.1 tree (davi:1.2609) BUG#21136

[Mats Kindahl]

Konstantin Osipov wrote:
> * Davi Arnaut <davi@stripped> [07/09/11 08:56]:
>
>   
>> diff -Nrup a/sql/sql_insert.cc b/sql/sql_insert.cc
>> --- a/sql/sql_insert.cc	2007-09-10 06:42:13 -03:00
>> +++ b/sql/sql_insert.cc	2007-09-11 01:29:29 -03:00
>> @@ -3265,7 +3265,7 @@ static TABLE *create_table_from_items(TH
>>                                        TABLE_LIST *create_table,
>>                                        Alter_info *alter_info,
>>                                        List<Item> *items,
>> -                                      MYSQL_LOCK **lock,
>> +                                      MYSQL_LOCK **extra_lock,
>>                                        TABLEOP_HOOKS *hooks)
>>  {
>>    TABLE tmp_table;		// Used during 'Create_field()'
>> @@ -3277,6 +3277,7 @@ static TABLE *create_table_from_items(TH
>>    Item *item;
>>    Field *tmp_field;
>>    bool not_used;
>> +  MYSQL_LOCK *lock;
>>    DBUG_ENTER("create_table_from_items");
>>  
>>    DBUG_EXECUTE_IF("sleep_create_select_before_check_if_exists",
> my_sleep(6000000););
>> @@ -3406,20 +3407,27 @@ static TABLE *create_table_from_items(TH
>>  
>>    table->reginfo.lock_type=TL_WRITE;
>>    hooks->prelock(&table, 1);                    // Call prelock hooks
>> -  if (! ((*lock)= mysql_lock_tables(thd, &table, 1,
>> -                                    MYSQL_LOCK_IGNORE_FLUSH, &not_used)) ||
>> +  if (! (lock= mysql_lock_tables(thd, &table, 1,
>> +                                 MYSQL_LOCK_IGNORE_FLUSH, &not_used)) ||
>>          hooks->postlock(&table, 1))
>>    {
>> -    if (*lock)
>> +    if (lock)
>>      {
>> -      mysql_unlock_tables(thd, *lock);
>> -      *lock= 0;
>> +      mysql_unlock_tables(thd, lock);
>> +      if (! (create_info->options & HA_LEX_CREATE_TMP_TABLE))
>> +        *extra_lock= 0;
>>      }
>>  
>>      if (!create_info->table_existed)
>>        drop_open_table(thd, table, create_table->db,
> create_table->table_name);
>>      DBUG_RETURN(0);
>>    }
>> +
>> +  if (create_info->options & HA_LEX_CREATE_TMP_TABLE)
>> +    mysql_unlock_tables(thd, lock);
>> +  else
>> +    *extra_lock= lock;
>> +
>>     
>
> You're on track, but I think we can't unlock prematurely even a
> temporary table - it's a violation of the storage engine protocol,
> which demands that write_row() is called under a lock, even for
> temporary tables.
>
> We need to rethink implementation of extra_lock, perhaps. And
> maybe it's not a #runtime but #replication problem, since
> extra_lock is used only to write table map events.
>
> Perhaps you could talk to Mats from replication team to see if
> thd->extra_lock could be removed altogether and a member of
> create_select class used instead?
>   

It is not that easy to remove, since the lock is an extra lock taken on 
the table that is created. The lock is needed in the generic code that 
generates table map event, and I don't think there is any easy way to 
remove the lock unless... hmm...

It might actually be possible to stuff that lock into the normal lock. 
The twist is that the table does not exist, which means taking a name 
lock. I don't know if you can take a name lock as part of a normal 
lock_tables sequence.

Need to think about it a little more.

Just my few cents,
Mats Kindahl
> Thanks.
>
>   


-- 
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com