Bug #30139 pid_file variable in my.cnf is not accepted
Submitted: 31 Jul 2007 9:55 Modified: 31 Jul 2007 12:32
Reporter: Oli Sennhauser Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: General Severity:S3 (Non-critical)
Version:5.0.45, 5.1 OS:Any
Assigned to: Assigned Account CPU Architecture:Any
Tags: Contribution
Triage: Triaged: D4 (Minor)

[31 Jul 2007 9:55] Oli Sennhauser
Description:
when the pid-file is explicitly set in the my.cnf as pid_file the parameter is ignored but pid-file works.
This does not meet our new naming convention with the "_".

How to repeat:
# cat my.cnf
[client]
port          = 3316
socket        = /home/mysql/tmp/mysql-3316.sock
[mysqld]
port          = 3316
socket        = /home/mysql/tmp/mysql-3316.sock
pid_file      = /tmp/mysql-3316.pid

# /home/mysql/product/mysql-5.0.45/bin/safe_mysqld --defaults-file=/home/mysql/product/mysql-5.0.45/data/my.cnf --ledir=/home/mysql/product/mysql-5.0.45/bin --language=/home/mysql/product/mysql-5.0.45/share/mysql/english --port=3316 --socket=/home/mysql/tmp/mysql-3316.sock --datadir=/home/mysql/product/mysql-5.0.45/data &

# ll -d /tmp/*.pid /home/mysql/product/mysql-5.0.45/data/*.pid
-rw-rw---- 1 mysql dba 5 2007-07-31 11:51 /home/mysql/product/mysql-5.0.45/data/master.pid

# cat my.cnf
[client]
port          = 3316
socket        = /home/mysql/tmp/mysql-3316.sock
[mysqld]
port          = 3316
socket        = /home/mysql/tmp/mysql-3316.sock
pid-file      = /tmp/mysql-3316.pid

# /home/mysql/product/mysql-5.0.45/bin/safe_mysqld --defaults-file=/home/mysql/product/mysql-5.0.45/data/my.cnf --ledir=/home/mysql/product/mysql-5.0.45/bin --language=/home/mysql/product/mysql-5.0.45/share/mysql/english --port=3316 --socket=/home/mysql/tmp/mysql-3316.sock --datadir=/home/mysql/product/mysql-5.0.45/data &

# ll -d /tmp/*.pid /home/mysql/product/mysql-5.0.45/data/*.pid
-rw-rw---- 1 mysql dba 5 2007-07-31 11:54 /tmp/mysql-3316.pid

Suggested fix:
accept also pid_file

Please also check all other variables for this behavior. I can remember this has happened already a few months ago with an other variable!
[31 Jul 2007 11:05] Sveta Smirnova
Thank you for the report.

How do you start mysqld?
[31 Jul 2007 11:11] Hartmut Holzgraefe
The problem seems to be that our helper shell scripts like mysqld_safe and mysqld_multi do not do any _/- conversion themselves, they just check for the - form and rely on my_print_defaults to extract my.cnf settings

So extending my_print_defaults to do the conversion (either always or on demand) should solve this (and bug #30140, too, which is most likely a duplicate falling into the same trap)
[31 Jul 2007 11:39] Hartmut Holzgraefe
the following patch to load_defaults() should fix this problem
by converting _ to - in option names. As load_defaults() returns
options in command line argument form anyway making this the default
behavior should be ok, no need to make this configurable ...

===== default.c 1.89 vs edited =====
--- 1.89/mysys/default.c        2007-07-31 13:38:14 +02:00
+++ edited/default.c    2007-07-31 13:38:01 +02:00
@@ -574,7 +574,7 @@
                                         const char *config_file,
                                         int recursion_level)
 {
-  char name[FN_REFLEN + 10], buff[4096], curr_gr[4096], *ptr, *end, **tmp_ext;
+  char name[FN_REFLEN + 10], buff[4096], curr_gr[4096], *ptr, *ptr2, *end, **tmp_ext;
   char *value, option[4096], tmp[FN_REFLEN];
   static const char includedir_keyword[]= "includedir";
   static const char include_keyword[]= "include";
@@ -736,6 +736,12 @@
     if (!value)
     {
       strmake(strmov(option,"--"),ptr,(uint) (end-ptr));
+
+      /* replace _ with - in option names for command line options */
+      for (ptr2 = option; *ptr2; ptr2++) 
+        if (*ptr2 == '_') 
+                 *ptr2 = '-';      
+
       if (opt_handler(handler_ctx, curr_gr, option))
         goto err;
     }
@@ -762,6 +768,12 @@
        value_end--;
       }
       ptr=strnmov(strmov(option,"--"),ptr,(uint) (end-ptr));
+
+      /* replace _ with - in option names for command line options */
+      for (ptr2 = option; ptr2 < ptr; ptr2++) 
+        if (*ptr2 == '_') 
+                 *ptr2 = '-';      
+
       *ptr++= '=';
 
       for ( ; value != value_end; value++)
[31 Jul 2007 12:32] Sveta Smirnova
Thank you for the report.

Verified as described. You have to use mysqld_safe or othe startup script to repeat the bug.
[31 Jul 2007 13:32] Magnus BlÄudd
The general idea for fixing the problem looks fine to me. But I think we could move the creation of the "option" to one common place in the function in order to avoid having the same code twice.

mysys/default.c:~732>>
    end= remove_end_comment(ptr);

<<< This is probably a good place to put the code that fills "option" variable with the option name(i.e --pid-file)

    if ((value= strchr(ptr, '=')))
      end= value;				/* Option without argument */
    for ( ; my_isspace(&my_charset_latin1,end[-1]) ; end--) ;
<<

Also a test would come handy, see mysqldump.test to see how to run test against my_print_defaults.
[19 Jun 2013 5:40] Simon Mudd
I have just realised that I have been bitten by this. I am sure (bad memory) that this worked before, and I used to use a relative expression pid_file = mysql.pid which was hostname agnostic so that if the hostname changed nothing would break. It seems my memory is wrong.

That said what concerns me is that the parameter is ignored completely. I see no error messages that it is being ignored and of course if you set this value to something different it can confuse the init scripts (in RH) and not shut down the server correctly, especially if the change takes place on a running system.

So please:
1. make mysqld complain if it does not like the pid_file entry
2. make it accept this entry
3. make it accept a relative path (as with relay_log, general_log_file, slow_query_log_file, etc.

So this has been seen in MySQL 5.5.23 and MySQL 5.6.12
[19 Jun 2013 7:29] Simon Mudd
To correct my previous comment it seems I had been using a pid_file setting in [mysqld_safe] and not [mysqld] as I should have.  However, as stated in the bug if you do set pid_file in [mysqld] it is silently ignored which is rather unexpected.  A good thing to fix in 5.7 perhaps.