Bug #59489 Enable setting of env. variables for mysqld from mtr
Submitted: 13 Jan 2011 23:43 Modified: 9 Feb 2011 19:22
Reporter: Mattias Jonsson Email Updates:
Status: Closed Impact on me:
None 
Category:Tools: MTR / mysql-test-run Severity:S4 (Feature request)
Version:5.5 OS:Any
Assigned to: Bjørn Munch CPU Architecture:Any

[13 Jan 2011 23:43] Mattias Jonsson
Description:
Using 'LD_PRELOAD=/usr/lib/libtcmalloc.so.0 HEAPPROFILE=/tmp/mysqld_mtr ./mtr 1st' does not work.

By adding '--mysqld-env' flags to mysql-test-run.pl, SafeProcess.pm and SafeProcess/Base.pm heap profile can be used by:

./mtr --mysqld-env="LD_PRELOAD=/usr/lib/libtcmalloc.so.0" --mysqld-env="HEAPPROFILE=/tmp/mtr_hprof_1st" 1st

How to repeat:
install google-perftools

LD_PRELOAD=/usr/lib/libtcmalloc.so.0 HEAPPROFILE=/tmp/mysqld_mtr ./mtr 1st

Suggested fix:
add a '--mysqld-env' variable which accumulates environment variables to be set before forking mysqld.

See attached patch.
[13 Jan 2011 23:46] Mattias Jonsson
patch for mysql-test-run.pl etc. to allow setting environment variables before spawning mysqld

Attachment: mysql-test-run.diff (text/x-diff), 3.04 KiB.

[13 Jan 2011 23:59] Mattias Jonsson
The patch will probably need some more work to be aligned with the code in mysql-test-run.pl etc.

Also it would be even better if it did not set the environment variables in create_process, but instead gave them as flags to my_safe_process and let my_safe_process set the environment between fork and exec to avoid being profiled itself, but that is not a big deal.

Another feature request would be to replace text in the --mysqld-env variable with $mysqld->name(), like running a master/slave test with --mysqld-env="HEAPPROFILE=/tmp/test_MYSQLDNAME" which would be set to /tmp/test_mysqld.1 for the master and tmp/test_mysqld.2 for the slave.
[17 Jan 2011 8:41] Bjørn Munch
OK I see this might be useful, but I'm not too keen on adding a new feature to 5.1 now. If you really have to run this on 5.1, a possible workaround is to replace mysqld with a small wrapper script that sets the envs and then exec's the real mysqld.
[17 Jan 2011 9:02] Bjørn Munch
I wondered why this didn't work so I did some test. If I set an env. variable in the shell from which I run mtr, that variable IS set for mysqld. This was tested on 5.1.

You can test yourself this way: mv the mysqld executable to another directory (don't change its name, only the directory, then replace it with an executable script like this:

#!/bin/sh
env > /tmp/mysqldenv
exec <noew directory>/mysqld $*

Now set some env. variable, run any test, and see what it printed to 
/tmp/mysqldenv.

Perhaps you need to 'export' those variables?
[18 Jan 2011 11:44] Mattias Jonsson
Here is what I get on Ubuntu 10.04 LTS server:
LD_PRELOAD=/usr/lib/libtcmalloc.so.0 HEAPPROFILE=/tmp/hptest ./mtr 1st
Logging: /home/mattiasj/clones/test-5.5/mysql-test/mysql-test-run.pl  1st
/bin/pwd: symbol lookup error: /usr/lib/libtcmalloc.so.0: undefined symbol: pthread_key_create
mysql-test-run: *** ERROR: Could not find any of ./share/mysql/charsets ./sql/share/charsets ./share/charsets
HeapProfiler: Dumping heap profile to /tmp/hptest_9603.0001.heap (Exiting)
HeapProfiler: Dumping heap profile to /tmp/hptest.0001.heap (Exiting)

So I cannot use the mysql-test-run.pl out of the box, but need to move the mysqld binary to another directory (a different name will fail version detection), and create a wrapper script like:
#!/bin/sh
LD_PRELOAD=/usr/lib/libtcmalloc.so.0 HEAPPROFILE=/tmp/hptest-wrapper exec /home/mattiasj/clones/test-5.5/b/sql/real/mysqld $*

This works as a workaround.

But it would be much more convenient if I could somehow do a one-liner including mysql-test-run.pl :)

Regarding not introducing it in 5.1, I don't have a strong opinion on which version to include it in, but since it is not a part of the production running binary, I cannot see the problem with including it in 5.1, unless it is a risk to introduce more failing tests.
[18 Jan 2011 11:52] Bjørn Munch
You need to 'export' those variables from your shell, only then do they get propagated.
[19 Jan 2011 15:29] Bjørn Munch
After discussing on IRC, it's clear that the problem arises because the env. setting we want to pass to mysqld is poisonous to other processes in the "stack". So there is a need for a feature like this. The env.var. should not be set until we actually start mysqld.
[20 Jan 2011 13:36] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/129278

3155 Bjorn Munch	2011-01-20
      Bug #59489 Enable setting of env. variables for mysqld from mtr
      Added --mysqld-env option, propagate via safe_process
      On Windows, we do set it in the parent. Not tested on Windows yet
[26 Jan 2011 11:34] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/129648

3159 Bjorn Munch	2011-01-26
      Bug #59489 Enable setting of env. variables for mysqld from mtr
      Added --mysqld-env option, propagate via safe_process
      On Windows, we do set it in the parent.
[27 Jan 2011 12:31] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/129756

3159 Bjorn Munch	2011-01-27
      Bug #59489 Enable setting of env. variables for mysqld from mtr
      Added --mysqld-env option, propagate via safe_process
      Simplified: should be safe to set in parent safe_process after it's started
[27 Jan 2011 12:36] Magnus Blåudd
Maybe also check what happens if you user forget to pass a string to the env var argument?

Valid:
safe_procecc --env MY_ENV=value

Invalid?
safe_process --env MY_ENV
safe_process --env
[27 Jan 2011 13:43] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/129767

3159 Bjorn Munch	2011-01-27
      Bug #59489 Enable setting of env. variables for mysqld from mtr
      Added --mysqld-env option, propagate via safe_process
      Simplified: should be safe to set in parent safe_process after it's started
      Addendum: catch cases of --mysqld-env w/o value, assume env.var 
          name never begins with "--"
[27 Jan 2011 18:36] Bjørn Munch
Pushed to 5.5-mtr and trunk-mtr
[30 Jan 2011 11:52] Bugs System
Pushed into mysql-trunk 5.6.2 (revid:bjorn.munch@oracle.com-20110130115129-js8oloj0kke23y2u) (version source revid:bjorn.munch@oracle.com-20110130115042-u671iduxcr5uw04c) (merge vers: 5.6.2) (pib:24)
[30 Jan 2011 11:53] Bugs System
Pushed into mysql-5.5 5.5.10 (revid:bjorn.munch@oracle.com-20110130110933-9rcq2ic58w06ae30) (version source revid:bjorn.munch@oracle.com-20110130110933-9rcq2ic58w06ae30) (merge vers: 5.5.10) (pib:24)
[30 Jan 2011 18:14] Paul DuBois
Changes to test suite. No changelog entry needed.