maria-developers team mailing list archive

[Alexey Botchkov <holyfoot@xxxxxxxxxxxx>]

Sergei,


11.05.2012 19:44, Sergei Golubchik wrote:
> === 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?

It's not blocked as we ceased incrementing the refresh_version in this case:

+    /* No need to close the open tables if we just set the readonly state */
+    if (!set_readonly_mode)
+      refresh_version++;			// Force close of open tables
+


Right, this test with the 'send set read_only' should be either removed 
or updated.

> Where are the tests for the changed functionality?

No specific tests yet.
I was to lazy to add the new test while i was not sure i did the correct 
thing. Hoped to get the quick answer.

> > === 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.

That's right, it' won't work with the mixed types of tables.
But as the customer didn't ask for that, maybe the simplest solution 
will do here.


> Back to my first question - where are the tests for this new task?

Will develop some ASAP.

HF