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

Support for .user.ini files (PHP) #160

Closed
avkarenow opened this issue Aug 13, 2018 · 9 comments
Closed

Support for .user.ini files (PHP) #160

avkarenow opened this issue Aug 13, 2018 · 9 comments
Labels
z-enhancement ⬆️ Product Enhancement

Comments

@avkarenow
Copy link

Hello,

.user.ini file is good for users without access to server configuration and it should allow changes only variables with PHP_INI_PERDIR/PHP_INI_ALL flags.

Now my users are using .user.ini to reconfigure their applications (php-fpm), so it will good to still have that possibility with Unit.

More about .user.ini: http://php.net/manual/en/configuration.file.per-user.php

Any chances for support for it?

@markkrj
Copy link

markkrj commented Sep 22, 2021

I think this is #543?
Maybe add 'Fixes #160' in the pull request to auto close this when/if it gets merged
Btw, @mar0x do you plan to merge it?

@mar0x
Copy link
Contributor

mar0x commented Sep 23, 2021

Hello,
I'm afraid #543 is not ready for merge yet. I've commented the patch.

@markkrj
Copy link

markkrj commented Sep 23, 2021

Regarding https://github.com/nginx/unit/pull/543/files#r714593715:
Thinking more about .user.ini, I don't know if you wanna allow live config this way. Maybe to follow what other PHP application servers (mostly apache2/mod_php and PHP-FPM) do and developers expect, but that implies checking the ttl on every request or at main loop and then checking and loading configs recursively (from app's root up to the requested php script) once the cache expires. That is the reason nginx does not have an .htaccess equivalent in the first place, but again, it is a matter of how closely you wanna follow PHP-FPM spec. It's an obvious performance trade-off.
Other option is to close this as wont fix and explain why, maybe with a benchmark, like this.

@VBart
Copy link
Contributor

VBart commented Sep 23, 2021

We definitely don't want additional syscalls on every request. While sometimes it may be handy, but the overhead isn't worth it.
I'm sure that most users don't really need it updated on each request and, at the same time, they don't realise the costs.

@markkrj
Copy link

markkrj commented Sep 23, 2021

It won't actually be updated on every request. TTL is checked on every request, and if expired, then it will be updated. But I'm mostly on side of performance. Just wanted to point out that it is a necessary trade-off if you wanna follow PHP-FPM spec...

@VBart
Copy link
Contributor

VBart commented Sep 23, 2021

TTL cache may work good or not. First of all, the cache isn't shared between processes. As a result, if you have 50 processes, then it can be up to 50 scans for each entry even within TTL. If the processes configured to scale dynamically, then situation will be even worse as each process starts up with a clean cache.

At the same time, I guess that if we just scan all the directory tree for .user.ini files and load them once at the moment of creating/updating/reloading the PHP app in Unit, then that won't give any additional overhead in runtime and will work well for almost all users. We can add it as an optional mode, that will work by default, unless a user wants the classic behaviour and turns it off in the Unit config. What do you think?

@markkrj
Copy link

markkrj commented Sep 23, 2021

I think it is a good approach. Reloading Unit is just a matter of doing an API call anyway...

@VBart VBart added the z-enhancement ⬆️ Product Enhancement label Sep 23, 2021
@alejandro-colomar
Copy link
Contributor

Ping.

Hello,
Are you interested in this feature? I'll close the PR, since the design was rejected. However, feel free to open a new one redesigned with the things talked in this issue.

Cheers,

Alex

@tippexs
Copy link
Contributor

tippexs commented Dec 1, 2022

Closed due to timeout

@tippexs tippexs closed this as completed Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
z-enhancement ⬆️ Product Enhancement
Projects
None yet
Development

No branches or pull requests

6 participants