Hi Sergey,
Sergey Petrunia, 29.01.2008 18:26:
...
> ChangeSet@stripped, 2008-01-29 20:25:33+03:00, sergefp@stripped +4 -0
> BUG#30221 "log_tables.test fails on Windows"
> The problem was that for LIMIT queries handler->index_end() was not called,
> and DS-MRR scan was not properly de-initialized, which caused leaks of file
> descriptors in MyISAM.
>
> Fix: Call ds_mrr.dsmrr_close() also in handler->reset().
This comment sounds as if ds_mrr.dsmrr_close() is called in
handler->index_end(). But I don't find it. Even if LIMIT queries would
call handler->index_end(), DS-MRR scan would not properly de-initialize.
In the clone I looked (up to ChangeSet@stripped, 2008-01-28
23:51:44-08:00, brian@zim.(none) +2 -0 /
brian@zim.(none)|ChangeSet|20080129075144|36991), dsmrr_close() is
called only at the end of a scan, when dsmrr_next() returns an error
like HA_ERR_END_OF_FILE.
Is a LIMIT query the only case where a scan is terminated before its
end? If so, please mention this in the comment.
Also I ask for an explanation of "also" in the "Fix:" sentence.
...
> diff -Nrup a/mysql-test/t/disabled.def b/mysql-test/t/disabled.def
> --- a/mysql-test/t/disabled.def 2008-01-19 22:14:21 +03:00
> +++ b/mysql-test/t/disabled.def 2008-01-29 20:25:10 +03:00
> @@ -14,7 +14,6 @@ concurrent_innodb : BUG#21579 200
> ##order_by : WL#2475: spetrunia producing ordered streams is not handled
> correctly
> federated_transactions : Bug#29523 Transactions do not work
> subselect2 : BUG#31464 SergeyP will fix
> -log_tables : BUG#31580 1007-10-13 SergeyP log_tables.test fails on all windows
> platforms in pushbuild
> show_check : Bug #32682 Test show_check fails in 6.0
> events : Bug#32664 events.test fails randomly
> lowercase_table3 : Bug#32667 lowercase_table3.test reports to error log
> diff -Nrup a/sql/handler.h b/sql/handler.h
> --- a/sql/handler.h 2008-01-11 01:08:49 +03:00
> +++ b/sql/handler.h 2008-01-29 20:25:10 +03:00
> @@ -2227,7 +2227,7 @@ public:
> typedef void (handler::*range_check_toggle_func_t)(bool on);
>
> DsMrr_impl()
> - : use_default_impl(TRUE) {};
> + : h2(NULL), use_default_impl(TRUE) {};
Hm. The clone I look at (see above) has a different signature for the
constructor. And it uses different initializers.
Unfortunately you do not explain, why you need to initialize h2. From
code reading I guess it is because you do now call dsmrr_close() in
reset() when no dsmrr_init() might have been called before. dsmrr_init()
uses "if (h2)" to decide what to reset.
If my above guess is true, then I wonder, why you don't initialize
save_read_set and save_write_set. They are used unconditionally in
dsmrr_close(). But they are initialized in dsmrr_init() only, as far as
I can see in my clone.
Overmore, even inside dsmrr_init() they can be used when the "error:"
label is jumped at from before their initialization.
Please add more explanations so that I understand what's going on. I
leave the bug "In review", assuming that I missed something. However, if
I found something that needs a fix, please reset the bug to "In progress".
Regards
Ingo
--
Ingo Strüwing, Senior Software Developer
MySQL GmbH, Dachauer Str. 37, D-80335 München
Geschäftsführer: Kaj Arnö - HRB München 162140
