maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxxx>]

Hi, Holyfoot!

Looks good to me!
See a couple of comments below.
Please push after fixing all that.

On May 17, holyfoot@xxxxxxxxxxxx wrote:
> ------------------------------------------------------------
> revno: 3518
> revision-id: holyfoot@xxxxxxxxxxxx-20120517131405-y12erfkgta2ym2pn
> parent: sanja@xxxxxxxxxxxxxxxx-20120507181437-1zl1v1uxje7v6994
> committer: Alexey Botchkov <holyfoot@xxxxxxxxxxxx>
> 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