mysqld_safe does not honour underscores in options like mysqld does

[Arjen Lentz]

mysqld accepts both dashes and underscores in server options, however mysqld_safe does not.
Since it's unclear from the user's perspective (and they shouldn't NEED to know!) which options get processed only by mysqld_safe, this can mean that they use an option in the wrong way but don't notice it.
Example... open_files_limit
When written like that, it mysqld_safe will not recognise it and thus not call ulimit -n
...
The option WILL be feed to mysqld so it shows the "correct" number in SHOW GLOBAL VARIABLES. Except, since ulimit wasn't called, it's not actually in effect!

See http://bugs.mysql.com/40368

[Erik Ljungstrom (erik-ibiblio)]

This ought to do it:

--- ./mysql-5.0.67-sail-erik-build/scripts/mysqld_safe 2008-10-20 02:13:55.000000000 +0100
+++ mysqld_safe 2008-10-28 11:19:51.000000000 +0000
@@ -63,6 +63,10 @@
   fi

   for arg do
+ substr_a=$(echo $arg | sed -e 's/\(...\)\(.*\)=\(.*\)/\2/');
+ substr_b=$(echo $substr_a | sed s/_/-/g);
+ arg=`echo $arg | sed s/$substr_a/$substr_b/g`
+
     case "$arg" in
       --skip-kill-mysqld*)
         KILL_MYSQLD=0;

It replaces all underscores with dashes, apart from the two initial dashes and any underscores trailing the = character

So --some_option=argument_to_option would become --some-option=argument_to_option

Can anyone see an issue with this?

[Erik Ljungstrom (erik-ibiblio)]

Unless there are any objections to that solution, I'll do a proper diff against the .sh script rather than its product and put it in my branch?

[Arjen Lentz (arjen-lentz)]

Hi Erik

On 29/10/2008, at 3:42 AM, Erik Ljungstrom wrote:
> Unless there are any objections to that solution, I'll do a proper
> diff
> against the .sh script rather than its product and put it in my
> branch?

My main concern with this particular patch (great fast involvement
btw, thanks!) is that it introduces a dependency on sed.
While I think sed will generally be present on any baseline install,
there are some varieties... are these sed lines generic enough to not
break anywhere? And what happens if sed does not exist or chucks an
error?

Can this somehow be resolved without non-shell foo?

I was thinking about simply double-listing each option that has a dash
in it, but there may be better ways.

Cheers,
Arjen.
--
Arjen Lentz, Director @ Open Query (http://openquery.com.au)
Training and Expertise for MySQL in Australia and New Zealand

OurDelta: free enhanced builds for MySQL @ http://ourdelta.org

[Arjen Lentz (arjen-lentz)]

Hi Erik

On 29/10/2008, at 8:02 AM, Arjen Lentz wrote:
> My main concern with this particular patch (great fast involvement
> btw, thanks!) is that it introduces a dependency on sed.

Sorry amend that - just had a look at the current mysqld_safe and it
already uses sed allover the place. So that's not a problem.

Can the sed lines be done shorter/simpler? That'd be my only q there.
And a # comment line above the sed foo explaining what it does and why.

Then, yes please submit a branch with a new patch. Please my patch
naming for bugfixes (in the BIG_TABLES patch).
If you can make it such that it will apply both to the stock MySQL one
as well as our already slightly modified one, we can use the patch as
well as submit it upstream.

Thanks!

Cheers,
Arjen.
--
Arjen Lentz, Director @ Open Query (http://openquery.com.au)
Training and Expertise for MySQL in Australia and New Zealand

OurDelta: free enhanced builds for MySQL @ http://ourdelta.org

[Erik Ljungstrom (erik-ibiblio)]

Pushed a patch to my branch. Suggestions to make it look smaller is still welcome! It applies cleanly to a 5.0.67 tree.

The sed lines should run on any unix like platform. At the very least the ones supported by MySQL, I've tried it on Linux, FreeBSD and Solaris.

Cheers,
Erik Ljungstrom

[Arjen Lentz (arjen-lentz)]

(Pasting your note from email)
> in revision 38 there's a slight alteration for Solaris (ironically enough not the sed line).
> Basically, instead of $() it's using backticks which is compatible with Solaris' /bin/sh

> But the patch you pasted works for any Linux distro and FreeBSD version
> conceived on this side of the moon landing.

Ok, great work - but... ;-)
I think we need to patch this in a different spot: extra/my_print_defaults.c
Then any other scripts/tools that use the same will also be fixed, rather than just mysqld_safe.
Also, some distros replace/patch mysqld_safe (have their own foo) but the my_print_defaults binary should always be around and get updated.

I would suggest we fix this in main() in or just before the loop that outputs the params:
  for (argument= arguments+1 ; *argument ; argument++)
    puts(*argument);

[Erik Ljungstrom (erik-ibiblio)]

I can see the value of adding a patch like that, but that wouldn't as far as I can see (just had a very quick glance) work for command line supplied arguments. So maybe use two patches?

[Arjen Lentz (arjen-lentz)]

Hi Erik

On 01/11/2008, at 3:52 AM, Erik Ljungstrom wrote:
> I can see the value of adding a patch like that, but that wouldn't as
> far as I can see (just had a very quick glance) work for command line
> supplied arguments. So maybe use two patches?

Euh you mean fed to the command line of mysqld_safe?
True, that. Did you have a look at mysqld_multi also?

By the way I did find where to tweak it in the my_print_default code
(mysys library actually so it'll be in all tools), just turns out I
need a new pointer variable to deal with things in the right spot. I
was trying to do without but all existing vars are in use at that point.

Cheers,
Arjen.
--
Arjen Lentz, Director @ Open Query (http://openquery.com.au)
Training and Expertise for MySQL in Australia and New Zealand

OurDelta: free enhanced builds for MySQL @ http://ourdelta.org

[Erik Ljungstrom (erik-ibiblio)]

Hi,

> True, that. Did you have a look at mysqld_multi also?

mysqld_multi doesn't take arguments that's affected by this, and quite happily passes anything on to mysqld.

> By the way I did find where to tweak it in the my_print_default code

I'm not sure if anything is actually required in my_print_defaults. Anything coming from there as far as mysqld_safe is concerned goes through parse_arguments() and will be fixed by the patch anyways. What other tools do you have in mind?
We would still need the mysqld_safe patch since command line arguments are separate from anything to do with my_print_defaults

Thanks,
Erik

[Arjen Lentz (arjen-lentz)]

In OurDelta builds from patchset d7.
(only the mysqld_safe fix in the end, the my_print_defaults doesn't appear to be used elsewhere relevant so this is sufficient)