Hello Marc,
Marc Alff a écrit, Le 05.06.2009 03:46:
> Guilhem Bichot wrote:
>> Marc Alff a écrit, Le 04.06.2009 19:38:
>>> Guilhem Bichot wrote:
>>>> Marc Alff a écrit, Le 28.05.2009 23:33:
>>>>> #At file:///home/malff/BZR-TREE/mysql-6.0-wl4876/ based on
>>>>> revid:marc.alff@stripped
>>>>>
>>>>> 3156 Marc Alff 2009-05-28
>>>>> WL#4876 Parse options before initializing mysys
> This was already implemented and pushed when I replied previously.
ok
> It seems some commit emails have been lost or delayed today, with some
> internals systems down.
Some of them have just arrived a few minutes ago anyway I have pulled
and am reading them from bzr history ("bzr qlog" or "bzr viz")
>>> Refactoring the command line options parsing for the logger is not part
>>> of the scope of this patch, but I think every option affecting logging
>>> eventually should be parsed earlier in the server, as well.
>> Agree on both points.
>>
>>> I tested manually errors (--performance_schema_max) or warnings
>>> (--performance_schema_max_cond=-1), but could not get that automated
>>> with MTR.
>> Why not do like those cmdline_bad tests you had?
>
> The tests did work for a simple fprintf(stderr, "<message>").
>
> Now that the real log is used, there are:
> - time stamps
> - other error messages from the server
> - path to files
> etc in the output, making this impossible to test with MTR.
>
> In fact, I don't see *any* test in the test scripts that checks that the
> *server* (not other things like mysql_admin) properly reads .cnf files,
> and there is not even a MYSQLD variable defined in MTR itself to invoke
> the server and test command line options.
Yes, that's a true fact.
But, we can change this easily, thanks to new features of mtr.
Here's a tested example which does what cmd_badline.test did:
# start server with bad option, output to file
--exec $MYSQLD_BOOTSTRAP_CMD --performance-schema-enabled=maybe >
$MYSQLTEST_VARDIR/tmp/perfschema_options.txt 2>&1 || true
# errors in output file can be parsed with Perl or LOAD DATA INFILE
perl;
use strict;
use warnings;
my $fname= "$ENV{'MYSQLTEST_VARDIR'}/tmp/perfschema_options.txt";
open(FILE, "<", $fname) or die;
my @lines= <FILE>;
# those two must be in the file for the test to pass
my @patterns=
("unknown variable 'performance-schema-enabled=maybe",
"Aborting");
foreach my $one_line (@lines)
{
foreach my $one_pattern (@patterns)
{
# print pattern, not line, to get time-independent output
print "Found: $one_pattern\n" if ($one_line =~ /$one_pattern/);
}
}
close FILE;
EOF
The result file looks like:
Found: unknown variable 'performance-schema-enabled=maybe
Found: Aborting
which checks that the option caused failure as expected.
Instead of "||true" (which I found in mysql_protocols.test), which I
wonder whether this works under Windows, you can use "--error 7" like in
mysqladmin.test.
And here is a tested example to test that properly spelled options in
my.cnf are seen by mysqld:
1) create <testname>.cnf containing for example
# Use default setting for mysqld processes
!include include/default_mysqld.cnf
[mysqld.1]
performance-schema-max-thread=318
(I took inspiration from other .cnf files in mysql-test).
2) then <testname>.test can just be:
select * from information_schema.GLOBAL_VARIABLES where VARIABLE_NAME =
'PERFORMANCE_SCHEMA_MAX_THREAD';
(or anything you prefer: SHOW ENGINE PERFORMANCE_SCHEMA STATUS...)
>>>>> + /* Remove the performance schema options parsed. */
>>>>> + defaults_argc= argc;
>>>>> + defaults_argv= argv;
>>>> GB load_defaults() has allocated memory (wityh alloc_root()) and
>>>> defaults_argv has to stay unchanged after that, so that free_defaults()
>>>> has the proper pointer to free. This is explained in
>>>> init_server_components() (look for "We need to eat").
>>>> handle_options() automatically does the removing of parsed P_S options;
>>>> you don't need to do anything for it.
>>>> Could you please also add a ".test" file to verify that there are no
>>>> problems here? It would start mysqld with a ".cnf" file
>>>> (--defaults-file=$MYSQLTEST_VARDIR/tmp/something.cnf, see
>>>> mysqladmin.test for an example) containing P_S options and then other
>>>> options. It would check that all options from .cnf" are well seen. It
>>>> can be modelled on your previous cmdline_bad_*.test test.
>>>>
>>> Fixed the free_defaults() problem, added clarity in the code.
>> And some test? After all, half of the reason for this WL#4876 is to have
>> P_S options of my.cnf be honoured, so we should check that they are...
>
> See my previous comments about MTR and the test suite.
With the suggestion above this can now be tested.
Thanks for all the work!!
--
Mr. Guilhem Bichot <guilhem@stripped>
Sun Microsystems / MySQL, Lead Software Engineer
Bordeaux, France
www.sun.com / www.mysql.com
