Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mysqlctld: allow setting flags / env vars for mysqld #5466

Closed
derekperkins opened this issue Nov 26, 2019 · 4 comments · Fixed by #5730
Closed

mysqlctld: allow setting flags / env vars for mysqld #5466

derekperkins opened this issue Nov 26, 2019 · 4 comments · Fixed by #5730
Labels
good first issue Good issue for new contributors Type: Enhancement Logical improvement (somewhere between a bug and feature)
Milestone

Comments

@derekperkins
Copy link
Member

We just added tcmalloc / jemalloc to the lite build #5444, but as currently constituted, they can't be easily used. malloc-lib is a mysqld_safe only option, and for systems without mysqld_safe like the default Percona images, there is no way to propagate LD_PRELOAD into the child mysqld process.

As discussed with @aquarapid at Kubecon, the only workaround available now is to move mysqld and set mysqld to execute a shell with LD_PRELOAD set, as shown below. It would be nice to have that inherit the parent environment or otherwise allow for variables to be set.

#!/bin/bash
export LD_PRELOAD="/usr/lib64/libtcmalloc.so.4"
/usr/sbin/mysqld_orig "$@"
@morgo morgo added the Type: Enhancement Logical improvement (somewhere between a bug and feature) label Nov 26, 2019
@morgo morgo added this to the v5.0 milestone Nov 26, 2019
@morgo
Copy link
Contributor

morgo commented Nov 26, 2019

It would be good to use an LD_PRELOAD for the testsuite as well #5007

@neowulf
Copy link
Contributor

neowulf commented Jan 10, 2020

I'd like to take up this issue. Is the following the right way to go about it?

  1. Is it just a matter of appending the environment variable LD_PRELOAD similar to how LD_LIBRARY_PATH is added to the environment before mysqld is executed?
  2. This change is environment specific. Did we want to support Mac OS by providing an alternate method?
  3. Preparation of the environment array can also be refactored into a separate function within mysqld.go

@deepthi
Copy link
Member

deepthi commented Jan 11, 2020

cc @morgo

@morgo
Copy link
Contributor

morgo commented Jan 11, 2020

I'd like to take up this issue. Is the following the right way to go about it?

  1. Is it just a matter of appending the environment variable LD_PRELOAD similar to how LD_LIBRARY_PATH is added to the environment before mysqld is executed?

Yes

  1. This change is environment specific. Did we want to support Mac OS by providing an alternate method?

The path will vary on linuxes too. So as long as we accept it as a full path, I believe it should be cross platform.

  1. Preparation of the environment array can also be refactored into a separate function within mysqld.go

I will let you use your judgement since it will be easier to understand when viewing the code. But yes - the code is getting a little complicated. I also think that it should do better checking and suggest that mysqld may not have started because of an apparmor / selinux issue. We recently adding checking inside the wrapping shellscripts: #5573 -- but I believe this should be done in the go program instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good issue for new contributors Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants