Hi, Holyfoot! On May 09, holyfoot@askmonty.org wrote:
------------------------------------------------------------ revno: 3518 revision-id: holyfoot@askmonty.org-20120509185945-i1yx6d38phgoqzes parent: sanja@montyprogram.com-20120507181437-1zl1v1uxje7v6994 committer: Alexey Botchkov <holyfoot@askmonty.org> branch nick: mdev-136 timestamp: Wed 2012-05-09 23:59:45 +0500 message: MDEV-136 Non-blocking "set read_only". Handle the 'set read_only=1' in lighter way, than the FLUSH TABLES READ LOCK; For the transactional engines we don't wait for operations on that tables to finish.
See my comments below.
=== modified file 'mysql-test/r/read_only.result' --- a/mysql-test/r/read_only.result 2009-03-06 14:56:17 +0000 +++ b/mysql-test/r/read_only.result 2012-05-09 18:59:45 +0000 @@ -59,7 +59,7 @@ connection con1; select @@global.read_only; @@global.read_only -0 +1
This is prone to race conditions. Please, fix the test to remove "send" here and below. (assuming the new result is correct) But is it correct ? Why set read_only is not blocked by a write locked myisam table?
unlock tables ; select @@global.read_only; @@global.read_only @@ -80,7 +80,7 @@ connection con1; select @@global.read_only; @@global.read_only -0 +1 unlock tables ; select @@global.read_only; @@global.read_only
Where are the tests for the changed functionality?
=== modified file 'sql/sql_base.cc' --- a/sql/sql_base.cc 2012-04-19 06:16:30 +0000 +++ b/sql/sql_base.cc 2012-05-09 18:59:45 +0000 @@ -933,7 +938,8 @@ for (uint idx=0 ; idx < open_cache.records ; idx++) { TABLE *table=(TABLE*) hash_element(&open_cache,idx); - if (table->in_use) + if (table->in_use && + (!set_readonly_mode || !table->file->has_transactions()))
I wonder how this could work. The line below sets a flag *on a thread*. The task description tells "not wait for transactional tables", while your change means "not set a flag if all tables used in a thread are transactional". That is, if a thread uses both transactional and non-transactional tables, your change does nothing. Back to my first question - where are the tests for this new task?
table->in_use->some_tables_deleted= 1; } }
Regards, Sergei