MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Daogang Qu Date:March 26 2010 6:18am
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (Dao-Gang.Qu:3001)
Bug#51851
View as plain text  
He Zhenxing wrote:
> Hi Daogang,
>
> Thank you for the work! But I'm afraid your patch is not correct. The
> lock has to be held while updating the table, ha_start_bulk_insert() and
> ha_end_bulk_insert() must be called before and after doing bulk
> (multi-row) insert. You patch will have problem when running two threads
> updating the same table in parallel. I think the original code in
> sql_load.cc is correct, please check if there are problems in the
> partition code.
>   
Thanks for the good comment. The partition code didn't release the
auto increment immediately after all the row are inserted, which
causes the problem. So the problem can be resolved by releasing the
auto increment immediately after all the row are inserted.
Please review the newest patch with the above solution. Thanks!

Best Regards,

Daogang
> Dao-Gang.Qu@stripped wrote:
>   
>> #At file:///home/daogangqu/mysql/bzrwork/bug51851/mysql-trunk-bugfixing/ based on
> revid:vvaintroub@stripped
>>
>>  3001 Dao-Gang.Qu@stripped	2010-03-23
>>       Bug #51851  Server with SBR locks mutex twice on LOAD DATA into
>>       partitioned MyISAM table
>>       
>>       In a multi-row insert statement like INSERT SELECT and LOAD DATA
>>       where the number of candidate rows to insert is not known in 
>>       advance we must hold a lock/mutex for the whole statement if we
>>       have statement based replication. Because the statement-based 
>>       binary log contains only the first generated value used by the
>>       statement, and slaves assumes all other generated values used by
>>       this statement were consecutive to this first one, we must 
>>       exclusively lock the generator until the statement is done. It 
>>       will be unlocked at the end of the statement by 
>>       ha_partition::release_auto_increment. During the process, the
>>       thread tries to lock it when updating the MyISAM table share info.
>>       So the mutex is locked twice on LOAD DATA into partitioned MyISAM
>>       table.
>>       
>>       To fix the problem, updating the MyISAM table share info before
>>       the lock is held by a multi-row insert statement.
>>      @ mysql-test/extra/rpl_tests/rpl_loaddata.test
>>         Added test case to verify whether server with SBR locks 
>>         mutex twice on LOAD DATA into partitioned MyISAM table.
>>      @ mysql-test/suite/ndb/r/ndb_load.result
>>         The test result is updated by the patch of bug#51851.
>>      @ mysql-test/suite/ndb/t/ndb_load.test
>>         Update the test to adapt the patch of Bug#51851.
>>      @ mysql-test/suite/rpl/r/rpl_loaddata.result
>>         Test result for the patch of Bug#51851.
>>      @ sql/sql_load.cc
>>         
>>
>>     modified:
>>       mysql-test/extra/rpl_tests/rpl_loaddata.test
>>       mysql-test/suite/ndb/r/ndb_load.result
>>       mysql-test/suite/ndb/t/ndb_load.test
>>       mysql-test/suite/rpl/r/rpl_loaddata.result
>>       sql/sql_load.cc
>> === modified file 'mysql-test/extra/rpl_tests/rpl_loaddata.test'
>> --- a/mysql-test/extra/rpl_tests/rpl_loaddata.test	2010-02-24 13:52:27 +0000
>> +++ b/mysql-test/extra/rpl_tests/rpl_loaddata.test	2010-03-23 08:51:22 +0000
>> @@ -264,4 +264,31 @@ SELECT * FROM t1;
>>  DROP TABLE t1;
>>  -- sync_slave_with_master
>>  
>> +#
>> +# Bug#51851  Server with SBR locks mutex twice on LOAD DATA into partitioned
> MyISAM table
>> +# This test verifies whether server with SBR locks mutex twice on LOAD DATA
>> +# into partitioned MyISAM table.
>> +#
>> +
>> +--connection master
>> +perl;
>> +open( INIT, ">init_file.txt");
>> +print INIT "1234\n";
>> +print INIT "abcd\n";
>> +close( INIT );
>> +EOF
>> +
>> +USE test;
>> +CREATE TABLE t1
>> +(id INT NOT NULL AUTO_INCREMENT PRIMARY KEY, name TINYBLOB NOT NULL,
>> +modified TIMESTAMP DEFAULT '0000-00-00 00:00:00',
>> +INDEX namelocs (name(255))) ENGINE = MyISAM
>> +PARTITION BY HASH(id) PARTITIONS 2;
>> +
>> +LOAD DATA LOCAL INFILE 'init_file.txt' INTO TABLE t1 (name);
>> +SELECT * FROM t1;
>> +REMOVE_FILE init_file.txt;
>> +DROP TABLE t1;
>> +--source include/master-slave-end.inc
>> +
>>  # End of 4.1 tests
>>
>> === modified file 'mysql-test/suite/ndb/r/ndb_load.result'
>> --- a/mysql-test/suite/ndb/r/ndb_load.result	2007-12-12 17:19:24 +0000
>> +++ b/mysql-test/suite/ndb/r/ndb_load.result	2010-03-23 08:51:22 +0000
>> @@ -1,7 +1,7 @@
>>  DROP TABLE IF EXISTS t1;
>>  CREATE TABLE t1 (word CHAR(20) NOT NULL PRIMARY KEY) ENGINE=NDB;
>>  LOAD DATA INFILE '../../../std_data/words.dat' INTO TABLE t1 ;
>> -ERROR 23000: Can't write; duplicate key in table 't1'
>> +ERROR 23000: Duplicate entry 'Aarhus' for key 'PRIMARY'
>>  DROP TABLE t1;
>>  CREATE TABLE t1 (word CHAR(20) NOT NULL) ENGINE=NDB;
>>  LOAD DATA INFILE '../../../std_data/words.dat' INTO TABLE t1 ;
>>
>> === modified file 'mysql-test/suite/ndb/t/ndb_load.test'
>> --- a/mysql-test/suite/ndb/t/ndb_load.test	2007-12-12 17:19:24 +0000
>> +++ b/mysql-test/suite/ndb/t/ndb_load.test	2010-03-23 08:51:22 +0000
>> @@ -11,7 +11,7 @@ DROP TABLE IF EXISTS t1;
>>  
>>  # should give duplicate key
>>  CREATE TABLE t1 (word CHAR(20) NOT NULL PRIMARY KEY) ENGINE=NDB;
>> ---error 1022
>> +--error 1062
>>  LOAD DATA INFILE '../../../std_data/words.dat' INTO TABLE t1 ;
>>  DROP TABLE t1;
>>  
>>
>> === modified file 'mysql-test/suite/rpl/r/rpl_loaddata.result'
>> --- a/mysql-test/suite/rpl/r/rpl_loaddata.result	2010-01-19 16:36:14 +0000
>> +++ b/mysql-test/suite/rpl/r/rpl_loaddata.result	2010-03-23 08:51:22 +0000
>> @@ -132,3 +132,15 @@ Field 3	'Field 4'
>>  'Field 5' 	'Field 6'
>>  Field 6	 'Field 7'
>>  DROP TABLE t1;
>> +USE test;
>> +CREATE TABLE t1
>> +(id INT NOT NULL AUTO_INCREMENT PRIMARY KEY, name TINYBLOB NOT NULL,
>> +modified TIMESTAMP DEFAULT '0000-00-00 00:00:00',
>> +INDEX namelocs (name(255))) ENGINE = MyISAM
>> +PARTITION BY HASH(id) PARTITIONS 2;
>> +LOAD DATA LOCAL INFILE 'init_file.txt' INTO TABLE t1 (name);
>> +SELECT * FROM t1;
>> +id	name	modified
>> +2	abcd	0000-00-00 00:00:00
>> +1	1234	0000-00-00 00:00:00
>> +DROP TABLE t1;
>>
>> === modified file 'sql/sql_load.cc'
>> --- a/sql/sql_load.cc	2010-02-24 17:04:00 +0000
>> +++ b/sql/sql_load.cc	2010-03-23 08:51:22 +0000
>> @@ -463,6 +463,35 @@ int mysql_load(THD *thd,sql_exchange *ex
>>                               (MODE_STRICT_TRANS_TABLES |
>>                                MODE_STRICT_ALL_TABLES)));
>>  
>> +    /*
>> +       Bug #51851  Server with SBR locks mutex twice on LOAD DATA into
>> +       partitioned MyISAM table
>> +
>> +       In a multi-row insert statement like INSERT SELECT and LOAD DATA
>> +       where the number of candidate rows to insert is not known in
>> +       advance we must hold a lock/mutex for the whole statement if we
>> +       have statement based replication. Because the statement-based
>> +       binary log contains only the first generated value used by the
>> +       statement, and slaves assumes all other generated values used by
>> +       this statement were consecutive to this first one, we must
>> +       exclusively lock the generator until the statement is done. It
>> +       will be unlocked at the end of the statement by
>> +       ha_partition::release_auto_increment. During the process, the
>> +       thread tries to lock it when updating the MyISAM table share info.
>> +       So the mutex is locked twice on LOAD DATA into partitioned MyISAM
>> +       table.
>> +
>> +       To fix the problem, updating the MyISAM table share info before
>> +       the lock is held by a multi-row insert statement.
>> +    */
>> +    int check_error= 0;
>> +    if (thd->locked_tables_mode <= LTM_LOCK_TABLES &&
>> +        table->file->ha_end_bulk_insert() && !error)
>> +    {
>> +      table->file->print_error(my_errno, MYF(0));
>> +      check_error= 1;
>> +    }
>> +
>>      if (ex->filetype == FILETYPE_XML) /* load xml */
>>        error= read_xml_field(thd, info, table_list, fields_vars,
>>                              set_fields, set_values, read_info,
>> @@ -475,12 +504,8 @@ int mysql_load(THD *thd,sql_exchange *ex
>>        error= read_sep_field(thd, info, table_list, fields_vars,
>>                              set_fields, set_values, read_info,
>>  			    *enclosed, skip_lines, ignore);
>> -    if (thd->locked_tables_mode <= LTM_LOCK_TABLES &&
>> -        table->file->ha_end_bulk_insert() && !error)
>> -    {
>> -      table->file->print_error(my_errno, MYF(0));
>> -      error= 1;
>> -    }
>> +    if (check_error && !error)
>> +      error= check_error;
>>      table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
>>      table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE);
>>      table->next_number_field=0;
>>
>>     
>
>
>
>   

Thread
bzr commit into mysql-trunk-bugfixing branch (Dao-Gang.Qu:3001) Bug#51851Dao-Gang.Qu23 Mar
  • Re: bzr commit into mysql-trunk-bugfixing branch (Dao-Gang.Qu:3001)Bug#51851He Zhenxing25 Mar
    • Re: bzr commit into mysql-trunk-bugfixing branch (Dao-Gang.Qu:3001)Bug#51851Daogang Qu26 Mar