saltstack-formulas / php-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
57 stars 231 forks source link

php-fpm reload by default #12

Closed giannello closed 5 years ago

giannello commented 10 years ago

The usual workflow with php-fpm is to trigger a reload instead of a restart, to gracefully handle for all the processes to terminate before killing and respawning the children.

Should we change the defaults for the php-fpm service to enforce this behavior?

TaiSHiNet commented 10 years ago

It's defaulted on pillar.example line 56. Altho I agree it should be defaulted on a lower level.

TaiSHiNet commented 10 years ago

https://bugs.launchpad.net/ubuntu/+source/php5/+bug/1242376

Be careful as currently, reload will leave non-working childs that need to be killed manually

And CC: @cheuschober just so you're up to speed with this

cheuschober commented 10 years ago

Thanks @TaiSHiNet, we mostly use Ubuntu here, hence why it's set-up the way it is. As noted, L56 of pillar.example does demonstrate a means for changing the default behavior of restart to reload for specific use-cases and that's a regular way of getting around things. Now, I could change that and ask Ubuntu users to do otherwise but it's a bit hairy. Feels like a good candidate for another grains filter-by. I think I owe @TaiSHiNet a few more changes to the formula but have been busy with other tasks. Probably see something near the end of the week.

cheuschober commented 10 years ago

I've made a few changes in the formula but am away from a testing environment right now.

Please test https://github.com/spsoit/php-formula/tree/fixes_unordered_fpm_params and see if it resolves your issues.

TaiSHiNet commented 10 years ago

I commented out the following lines:

 php:
  ng:
    fpm:
      service:
        enabled: True
#        opts:
#          reload: False

pushed a change and it restarted the daemon (instead of reloading it)

cheuschober commented 10 years ago

Which OS are you using? It should be restarting Ubuntu and reloading others.

TaiSHiNet commented 10 years ago

Ubuntu, which means the result (at least for me) was satisfactory

n-rodriguez commented 5 years ago

@myii I think we can close this one

myii commented 5 years ago

Closing this since it is a very old report. Feel free to reopen this if this is still an issue with the latest version of this formula.

@n-rodriguez, thanks for highlighting this.