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

Config reloading in Flying Passenger is broken #1554

Open
FooBarWidget opened this issue Jul 3, 2015 · 2 comments
Open

Config reloading in Flying Passenger is broken #1554

FooBarWidget opened this issue Jul 3, 2015 · 2 comments

Comments

@FooBarWidget
Copy link
Member

FooBarWidget commented Jul 3, 2015

It looks like the Passenger 5 pool options cache, which was meant to improve performance, broke config reloading in Flying Passenger. Because most options are now cached, reloading the web server configuration has no effect.

We should introduce a command for clearing that cache.

Furthermore, it looks like environment variables set with passenger_env_var are cached and never updated. We should clear that cache too.

@FooBarWidget FooBarWidget added this to the 5.1.0 milestone Jul 3, 2015
@OnixGH OnixGH removed this from the 5.1.0 milestone Sep 26, 2016
@CamJN CamJN added this to the 6.0.3 milestone Mar 7, 2019
@CamJN
Copy link
Contributor

CamJN commented Mar 7, 2019

High level process: finish converting components to config kit, and then expose an api for triggering a config reload & apply to configkit.

Details:

  • configkit already supports update transactions, just need a way to trigger one.
  • application pool is the main component not using config kit.
  • the app pool has the job of creating app descriptions that don't exist or looking up existing ones (keyed by group name).
  • the core receives the group name in a "!~" header, and looks up the app definition in the map
  • currently the core also receives a bunch of other settings in the headers that are duplicated in the config manifest (from the fuse panel stuff).
  • we should unify the source of configs (only the config manifest knows about all apps ahead of time, so that's the one to use)
  • ideally the core would get the group name in the header and that's it, and then lookup the app in the dict which would be populated via the config manifest.
  • currently the app pool when creating an app definition copies out settings and uses them to set other things, instead of referring to config kit (config kit has a cache, it's fast enough to use every time)
  • not blocking, but either the generation or parsing of the config manifest is slow if there are many virtualhosts (see: Excessive Apache startup time when running many virtual hosts/rails applications #2098)

@FooBarWidget
Copy link
Member Author

FooBarWidget commented Mar 7, 2019

The structure that currently best represents per-app config, is ApplicationPool::Options. However this structure is incomplete: it only represents config that are relevant to ApplicationPool. So for example PassengerMonitorLogFile is not represented inside that structure, which is why in the AdminPanelConnector you had to hack around it by querying configGetter and inspecting the JSON.

Furthermore, there are two dictionaries that map the app group name to ApplicationPool::Options. There is one in Core::Controller (the poolOptionsCache, see Controller::initializePoolOptions()). There is another one inside ApplicationPool::Pool. Currently there is a function ApplicationPool::Group::mergeOptions() which tries to merge any relevant (and changeable) differences between the two, but this entire approach is quite a nasty hack and I don't recommend continuing to use this approach.

I'll leave it up to you to think of a proper design. The easiest way would be to cram everything into ApplicationPool::Options, but that would violate separation-of-concerns. I haven't given a good thought yet about what would be a more elegant design, but that will no doubt be a bigger effort.

@CamJN CamJN removed this from the 6.0.3 milestone May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants