Hi,
Thanks for your quick review.
On 23.06.2007, at 11:58, Konstantin Osipov wrote:
> * kgeorge@stripped <kgeorge@stripped> [07/06/22 20:49]:
>> ChangeSet@stripped, 2007-06-22 19:20:55+03:00, gkodinov@stripped
>> +4 -0
>> Bug #29154: LOCK TABLES is not atomic when >1 InnoDB tables are
>> locked
>> LOCK TABLES takes a list of tables to lock. It may lock several
>> tables successfully and then encounter a tables that it can't
>> lock,
>> e.g. because it's locked. In such case it needs to undo the
>> locks on
>> the already locked tables. And it does that. But it has also
>> notified
>> the relevant table storage engine handlers that they should lock.
>> The only reliable way to ensure that the table handlers will
>> give up
>> their locks is to end the transaction. This is what UNLOCK TABLE
>> does : it ends the transaction if there were locked tables by
>> LOCK
>> tables.
>> It is possible to end the transaction when the lock fails in
>> LOCK TABLES because LOCK TABLES ends the transaction at its start
>> already.
>> Fixed by ending (again) the transaction when LOCK TABLES fails to
>> lock a table.
<snip>
>> +CONNECT (c1,localhost,root,,);
>> +CONNECT (c2,localhost,root,,);
>> +
>> +CONNECTION c1;
>
> Wow, I didn't know this syntax is allowed.
> I've always been using --connection.
>
> Could you precede each switch of the current connection with
> --echo switch to connection XXX
> ?
>
> That way it's easier to parse the result log.
Good point : will make it a rule when adding threaded tests. Done.
>> +SET AUTOCOMMIT=0;
>> +INSERT INTO t2 VALUES (1);
>> +
>> +CONNECTION c2;
>> +SET AUTOCOMMIT=0;
>> +--error ER_LOCK_WAIT_TIMEOUT
>> +LOCK TABLES t1 READ, t2 READ;
>> +
>> +connection c1;
>> +COMMIT;
>> +INSERT INTO t1 VALUES (1);
>> +
>> +CONNECTION default;
>> +SET AUTOCOMMIT=default;
>> +DISCONNECT c1;
>> +DISCONNECT c2;
>> +DROP TABLE t1,t2;
>
> I don't understand the layout of InnoDB tests.
> We have innodb-lock.test file. Perhaps your test should go there
> instead?
I was told that the innodb_mysql.test is the only innodb related test
where we can add innodb tests. The rest of the innodb*.test files are
for the InnoDB guys to handle.
> innondb_mysql.test seems to source mix1.inc and that's all - AFAIU
> this has been done to factor out some tests to use with Falcon in
> 6.0.
> If so, then maybe your test should go to mix1.inc file instead?
Right, I'll do that when I merge to 5.1, but for 5.0 there's no mix.inc.
> Finally, there is already innodb_mysql-master.opt. Why was it
> necessary to add yet another .opt file?
It was a duplicate indeed. Removed. Thanks for spotting that out.
Best Regards,
Joro
--
Georgi Kodinov, Senior Software Engineer
MySQL AB, Plovdiv, Bulgaria, www.mysql.com
Office: +359 32 634 397 Mobile: +359 887 700 566 Skype: georgekodinov
Are you MySQL certified? www.mysql.com/certification
