-
Notifications
You must be signed in to change notification settings - Fork 8k
Fixed bug #74631 (PDO_PCO with PHP-FPM: OCI environment initialized before PHP-FPM sets it up) #2534
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
Conversation
…efore PHP-FPM sets it up)
|
/cc @tianfyang: can you review? Maybe a delayed setup similar to PHP OCI8's is more efficient? |
|
Hi @KiNgMaR the problem is when running PHP-FPM, the process doesn't load environment, right? Can you please try if this workaround works for you? To me, the issue seems to be a PHP-FPM bug rather than a PDO_OCI bug. Your changes can fix this issue, but it will impact the performance as PDO_OCI would have to create OCI Env handle for each process instead of having only one OCI Env handle in the lifetime of the module. |
|
IMHO, this is not a bug. Indeed, environment need to be set before FPM is launched. Linux Distro, for years, use the /etc/sysconfig/php-fpm (loaded from SysV init script or EnvironmentFile systemd unit file) for such need |
|
It definitely is a bug. It is only noticeable for the To summarize, necessary prerequisites to run into the bug:
Symptoms: Possible fix: |
|
Also, in response to @remicollet, |
|
@KiNgMaR you can still set clear_env = yes in the FPM config file. The workaround I mentioned above is to set environment variables needed for loading modules like PDO_OCI before FPM is started. With the workaround above, you can set NLS settings in the . /home/user/env.sh, so that PDO_OCI can pick up the settings when is loaded. In php-fpm.config, set clear_env = yes to avoid the worker process accessing the shell environment variables and set your own environment variables for worker processes. |
|
Hi @tianfyang, that should be a valid workaround. However, in my testing it was not picking up |
|
Hi @KiNgMaR , we're still deciding whether we should merge this fix in. I've tested the your code change and it looks good. The only concern is the performance impact. I understand OCI8 is using the same way but for PDO_OCI user, it will be a performance degradation. Will let you know our decision soon. Just out of curiosity, you seem to be using both OCI8 and PDO_OCI as you've submitted another PR for PDO_OCI (merged). Any particular reason for using both drivers? |
|
Hi @tianfyang,
|
|
Hi @KiNgMaR We have decided it as a bug and will merge this fix in soon. Doing the initialization in |
|
Comment on behalf of tianfyan at php.net: Merged. Thanks for your contribution. |
https://bugs.php.net/bug.php?id=74631