Pass maxInstancesPerApp via a global, so that it is respected again.

Closes GH-2059. Also explain the various shortcuts taken.
phusion · May 22, 2018 · 6120509 · 6120509
1 parent 722d94e
commit 6120509
Show file tree

Hide file tree

Showing 9 changed files with 30 additions and 0 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,7 @@
 Release 5.3.2 (Not yet released)
 -------------
 
+ * Fixes PassengerMaxInstancesPerApp (Apache integration) not being respected (regression from config refactor in 5.2.0). Closes GH-2059.
  * [Enterprise] Fixes PassengerMaxInstances (Apache integration) not being respected (regression from config refactor in 5.2.0). 
 
 

diff --git a/dev/configkit-schemas/index.json b/dev/configkit-schemas/index.json
@@ -267,6 +267,10 @@
          "read_only" : true,
          "type" : "string"
       },
+      "max_instances_per_app" : {
+         "read_only" : true,
+         "type" : "unsigned integer"
+      },
       "min_spare_clients" : {
          "default_value" : 0,
          "has_default_value" : "static",
@@ -724,6 +728,10 @@
          "has_default_value" : "static",
          "type" : "any"
       },
+      "max_instances_per_app" : {
+         "read_only" : true,
+         "type" : "unsigned integer"
+      },
       "max_pool_size" : {
          "default_value" : 6,
          "has_default_value" : "static",
@@ -1441,6 +1449,10 @@
          "has_default_value" : "static",
          "type" : "any"
       },
+      "max_instances_per_app" : {
+         "read_only" : true,
+         "type" : "unsigned integer"
+      },
       "max_pool_size" : {
          "default_value" : 6,
          "has_default_value" : "static",

diff --git a/src/agent/Core/ApplicationPool/Group/StateInspection.cpp b/src/agent/Core/ApplicationPool/Group/StateInspection.cpp
@@ -70,6 +70,8 @@ Group::processLowerLimitsSatisfied() const {
  */
 bool
 Group::processUpperLimitsReached() const {
+	// check maxInstances limit as set by Enterprise (OSS maxInstancesPerApp piggybacks on this,
+	// see InitRequest.cpp)
 	return options.maxProcesses != 0 && capacityUsed() >= options.maxProcesses;
 }
 

diff --git a/src/agent/Core/Config.h b/src/agent/Core/Config.h
@@ -140,6 +140,7 @@ using namespace std;
  *   integration_mode                                                string             -          default("standalone")
  *   log_level                                                       string             -          default("notice")
  *   log_target                                                      any                -          default({"stderr": true})
+ *   max_instances_per_app                                           unsigned integer   -          read_only
  *   max_pool_size                                                   unsigned integer   -          default(6)
  *   multi_app                                                       boolean            -          default(false),read_only
  *   passenger_root                                                  string             required   read_only

diff --git a/src/agent/Core/Controller/Config.h b/src/agent/Core/Controller/Config.h
@@ -107,6 +107,7 @@ parseControllerBenchmarkMode(const StaticString &mode) {
  *   default_user                                        string             -          default("nobody")
  *   graceful_exit                                       boolean            -          default(true)
  *   integration_mode                                    string             -          default("standalone"),read_only
+ *   max_instances_per_app                               unsigned integer   -          read_only
  *   min_spare_clients                                   unsigned integer   -          default(0)
  *   multi_app                                           boolean            -          default(true),read_only
  *   request_freelist_limit                              unsigned integer   -          default(1024)
@@ -127,6 +128,8 @@ class ControllerSchema: public ServerKit::HttpServerSchema {
 	void initialize() {
 		using namespace ConfigKit;
 
+		add("max_instances_per_app", UINT_TYPE, OPTIONAL | READ_ONLY);
+
 		add("thread_number", UINT_TYPE, REQUIRED | READ_ONLY);
 		add("multi_app", BOOL_TYPE, OPTIONAL | READ_ONLY, true);
 		add("turbocaching", BOOL_TYPE, OPTIONAL | READ_ONLY, true);
@@ -307,6 +310,7 @@ class ControllerMainConfig {
 	unsigned int responseBufferHighWatermark;
 	StaticString integrationMode;
 	StaticString serverLogName;
+	unsigned int maxInstancesPerApp;
 	ControllerBenchmarkMode benchmarkMode: 3;
 	bool singleAppMode: 1;
 	bool userSwitching: 1;
@@ -324,6 +328,7 @@ class ControllerMainConfig {
 		  responseBufferHighWatermark(config["response_buffer_high_watermark"].asUInt()),
 		  integrationMode(psg_pstrdup(pool, config["integration_mode"].asString())),
 		  serverLogName(createServerLogName()),
+		  maxInstancesPerApp(config["max_instances_per_app"].asUInt()),
 		  benchmarkMode(parseControllerBenchmarkMode(config["benchmark_mode"].asString())),
 		  singleAppMode(!config["multi_app"].asBool()),
 		  userSwitching(config["user_switching"].asBool()),

diff --git a/src/agent/Core/Controller/InitRequest.cpp b/src/agent/Core/Controller/InitRequest.cpp
@@ -371,6 +371,12 @@ Controller::createNewPoolOptions(Client *client, Request *req,
 	fillPoolOption(req, options.fileDescriptorUlimit, "!~PASSENGER_APP_FILE_DESCRIPTOR_ULIMIT");
 	fillPoolOption(req, options.raiseInternalError, "!~PASSENGER_RAISE_INTERNAL_ERROR");
 	fillPoolOption(req, options.lveMinUid, "!~PASSENGER_LVE_MIN_UID");
+
+	// maxProcesses is configured per-application by the (Enterprise) maxInstances option (and thus passed
+	// via request headers). In OSS the max processes can also be configured, but on a global level
+	// (i.e. the same for all apps) using the maxInstancesPerApp option. As an easy implementation shortcut
+	// we apply maxInstancesPerApp to options.maxProcesses (which can be overridden by Enterprise).
+	options.maxProcesses = mainConfig.maxInstancesPerApp;
 	/******************/
 
 	boost::shared_ptr<Options> optionsCopy = boost::make_shared<Options>(options);

diff --git a/src/agent/Watchdog/Config.h b/src/agent/Watchdog/Config.h
@@ -132,6 +132,7 @@ using namespace std;
  *   integration_mode                                                         string             -          default("standalone")
  *   log_level                                                                string             -          default("notice")
  *   log_target                                                               any                -          default({"stderr": true})
+ *   max_instances_per_app                                                    unsigned integer   -          read_only
  *   max_pool_size                                                            unsigned integer   -          default(6)
  *   multi_app                                                                boolean            -          default(false),read_only
  *   passenger_root                                                           string             required   read_only

diff --git a/src/apache2_module/Hooks.cpp b/src/apache2_module/Hooks.cpp
@@ -1326,6 +1326,7 @@ class Hooks {
 		config["show_version_in_header"] = serverConfig.showVersionInHeader;
 		config["max_pool_size"] = serverConfig.maxPoolSize;
 		config["pool_idle_time"] = serverConfig.poolIdleTime;
+		config["max_instances_per_app"] = serverConfig.maxInstancesPerApp;
 		config["response_buffer_high_watermark"] = serverConfig.responseBufferHighWatermark;
 		config["stat_throttle_rate"] = serverConfig.statThrottleRate;
 		config["turbocaching"] = serverConfig.turbocaching;

diff --git a/src/nginx_module/ngx_http_passenger_module.c b/src/nginx_module/ngx_http_passenger_module.c
@@ -372,6 +372,7 @@ start_watchdog(ngx_cycle_t *cycle) {
     psg_json_value_set_ngx_str_ne(w_config, "default_ruby", &passenger_main_conf.default_ruby);
     psg_json_value_set_ngx_uint  (w_config, "max_pool_size", autogenerated_main_conf->max_pool_size);
     psg_json_value_set_ngx_uint  (w_config, "pool_idle_time", autogenerated_main_conf->pool_idle_time);
+    psg_json_value_set_ngx_uint  (w_config, "max_instances_per_app", autogenerated_main_conf->max_instances_per_app);
     psg_json_value_set_ngx_uint  (w_config, "response_buffer_high_watermark", autogenerated_main_conf->response_buffer_high_watermark);
     psg_json_value_set_ngx_uint  (w_config, "stat_throttle_rate", autogenerated_main_conf->stat_throttle_rate);
     psg_json_value_set_ngx_str_ne(w_config, "admin_panel_url", &autogenerated_main_conf->admin_panel_url);