Re: [Maria-developers] MDEV-136 Non-blocking "set read_only"

Hi, Holyfoot! Looks good to me! See a couple of comments below. Please push after fixing all that. On May 17, holyfoot@askmonty.org wrote:

------------------------------------------------------------ revno: 3518 revision-id: holyfoot@askmonty.org-20120517131405-y12erfkgta2ym2pn parent: sanja@montyprogram.com-20120507181437-1zl1v1uxje7v6994 committer: Alexey Botchkov branch nick: mdev-136 timestamp: Thu 2012-05-17 18:14:05 +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.

=== modified file 'mysql-test/t/read_only_innodb.test' --- a/mysql-test/t/read_only_innodb.test 2008-04-08 05:20:58 +0000 +++ b/mysql-test/t/read_only_innodb.test 2012-05-17 13:14:05 +0000 @@ -75,7 +75,28 @@ SELECT * FROM t1; COMMIT;

-connection default; +# +# Tests that LOCK TABLE doesn't block the SET READ_ONLY=1 for the InnoDB tables +# +connection default; +UNLOCK TABLES; + +--echo connection con1; +connection con1; +lock table t1 write;

See whether you can directly test that the table was not reopened. Perhaps by examining the corresponding SHOW STATUS variables you could do that.

+ +--echo connection default; +connection default; +set global read_only=1; + +--echo connection con1; +connection con1; +unlock tables; + +--echo connection default; +connection default; +SET GLOBAL read_only=0; + UNLOCK TABLES; DROP TABLE t1; DROP USER test@localhost; === 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-17 13:14:05 +0000 @@ -933,7 +938,14 @@ for (uint idx=0 ; idx < open_cache.records ; idx++) { TABLE *table=(TABLE*) hash_element(&open_cache,idx); - if (table->in_use) + /* We don't increment the refresh_version when set_readonly_mode, */ + /* but we still need non-transactional tables to be reopened. */ + /* So we set their versions as 'refresh_version - 1', which marks */ + /* them for the 'needs_reopen_or_table_lock()' */

please, use the standard mysql comment style as you can see everywhere in the sources

+ if (set_readonly_mode && !table->file->has_transactions()) + table->s->version= refresh_version-1;

better set it to 0, that's what tdc_remove_table() does. And we won't need to think about possible race conditions.

+ if (table->in_use && + (!set_readonly_mode || !table->file->has_transactions())) table->in_use->some_tables_deleted= 1; } }

Regards, Sergei