add a flag for using old routing

CamJN · CamJN · commit 484f843701ba · 2025-01-23T11:27:55.000-07:00
diff --git a/CHANGELOG b/CHANGELOG
@@ -4,6 +4,10 @@ Release 6.0.25 (Not yet released)
  * [Standalone] Adds a config option to specify the stop timeout for Passenger: `--stop-timeout 120` or `PASSENGER_STOP_TIMEOUT=120`.
  * [Standalone] Changes Passenger's (not apps') start timeout to 25s (from 15s), stop timeouts default to 60s.
  * [Ruby] Fixes an issue where Bundler would try to re-exec the process name instead of the script. Closes GH-2567 and GH-2577.
+ * [Enterprise] Adds a temporary flag to allow reverting to previous routing behaviour, in order to mitigate possible performance regressions, this flag will become a no-op and eventually removed once the routing issues have been fixed. Closes GH-2579.
+   - Apache: PassengerOldRouting on
+   - Nginx: passenger_old_routing on;
+   - Standalone: --old-routing
  * Updated various library versions used in precompiled binaries (used for e.g. gem installs):
    - cmake: 3.31.2 -> 3.31.3
    - curl: 8.11.0 -> 8.11.1
diff --git a/dev/configkit-schemas/index.json b/dev/configkit-schemas/index.json
@@ -302,6 +302,12 @@
          "read_only" : true,
          "type" : "boolean"
       },
+      "old_routing" : {
+         "default_value" : false,
+         "has_default_value" : "static",
+         "read_only" : true,
+         "type" : "boolean"
+      },
       "request_freelist_limit" : {
          "default_value" : 1024,
          "has_default_value" : "static",
@@ -806,6 +812,12 @@
          "read_only" : true,
          "type" : "boolean"
       },
+      "old_routing" : {
+         "default_value" : false,
+         "has_default_value" : "static",
+         "read_only" : true,
+         "type" : "boolean"
+      },
       "oom_score" : {
          "read_only" : true,
          "type" : "string"
@@ -1690,6 +1702,12 @@
          "read_only" : true,
          "type" : "boolean"
       },
+      "old_routing" : {
+         "default_value" : false,
+         "has_default_value" : "static",
+         "read_only" : true,
+         "type" : "boolean"
+      },
       "passenger_root" : {
          "read_only" : true,
          "required" : true,
diff --git a/resources/templates/standalone/http.erb b/resources/templates/standalone/http.erb
@@ -32,6 +32,7 @@ passenger_user_switching off;
 <%= nginx_http_option(:pool_idle_time) %>
 <%= nginx_http_option(:max_preloader_idle_time) %>
 <%= nginx_http_option(:turbocaching) %>
+<%= nginx_http_option(:old_routing) %>
 <%= nginx_http_option(:instance_registry_dir) %>
 <%= nginx_http_option(:spawn_dir) %>
 <%= nginx_http_option(:disable_security_update_check) %>
diff --git a/src/agent/Core/ApplicationPool/Group.h b/src/agent/Core/ApplicationPool/Group.h
@@ -27,8 +27,6 @@
 #define _PASSENGER_APPLICATION_POOL2_GROUP_H_
 
 #include <string>
-#include <map>
-#include <queue>
 #include <deque>
 #include <boost/thread.hpp>
 #include <boost/bind/bind.hpp>
@@ -202,6 +200,8 @@ class Group: public boost::enable_shared_from_this<Group> {
 	Callback shutdownCallback;
 	GroupPtr selfPointer;
 
+	// Whether to use the old routing algorithm
+	bool oldRouting;
 
 	/****** Initialization and shutdown ******/
 
diff --git a/src/agent/Core/ApplicationPool/Group/InitializationAndShutdown.cpp b/src/agent/Core/ApplicationPool/Group/InitializationAndShutdown.cpp
@@ -136,6 +136,7 @@ Group::Group(Pool *_pool, const Options &_options)
 	}
 
 	detachedProcessesCheckerActive = false;
+	oldRouting = _options.oldRouting;
 }
 
 Group::~Group() {
diff --git a/src/agent/Core/ApplicationPool/Group/SessionManagement.cpp b/src/agent/Core/ApplicationPool/Group/SessionManagement.cpp
@@ -29,9 +29,7 @@
 #ifdef INTELLISENSE
 	#include <Core/ApplicationPool/Pool.h>
 #endif
-#include <Shared/Fundamentals/Utils.h>
 #include <Core/ApplicationPool/Group.h>
-#include <cassert>
 
 /*************************************************************************
  *
@@ -361,7 +359,7 @@ Group::get(const Options &newOptions, const GetCallback &callback,
 
 bool
 Group::useNewRouting() const {
-	return !Agent::Fundamentals::getEnvBool("PASSENGER_OLD_ROUTING", false);
+	return !oldRouting;
 }
 
 } // namespace ApplicationPool2
diff --git a/src/agent/Core/ApplicationPool/Options.h b/src/agent/Core/ApplicationPool/Options.h
@@ -28,7 +28,6 @@
 
 #include <string>
 #include <vector>
-#include <utility>
 #include <boost/shared_array.hpp>
 #include <WrapperRegistry/Registry.h>
 #include <DataStructures/HashedStaticString.h>
@@ -79,7 +78,7 @@ class Options {
 	template<typename OptionsClass, typename StaticStringClass>
 	static vector<StaticStringClass *> getStringFields(OptionsClass &options) {
 		vector<StaticStringClass *> result;
-		result.reserve(20);
+		result.reserve(30);
 
 		result.push_back(&options.appRoot);
 		result.push_back(&options.appGroupName);
@@ -443,6 +442,8 @@ class Options {
 
 	/*********************************/
 
+	bool oldRouting;
+
 	/**
 	 * Creates a new Options object with the default values filled in.
 	 * One must still set appRoot manually, after having used this constructor.
@@ -480,7 +481,8 @@ class Options {
 		  statThrottleRate(DEFAULT_STAT_THROTTLE_RATE),
 		  maxRequests(0),
 		  currentTime(0),
-		  noop(false)
+		  noop(false),
+		  oldRouting(false)
 		  /*********************************/
 	{
 		/*********************************/
diff --git a/src/agent/Core/Config.h b/src/agent/Core/Config.h
@@ -155,6 +155,7 @@ using namespace std;
  *   max_instances_per_app                                           unsigned integer   -          read_only
  *   max_pool_size                                                   unsigned integer   -          default(6)
  *   multi_app                                                       boolean            -          default(false),read_only
+ *   old_routing                                                     boolean            -          default(false),read_only
  *   oom_score                                                       string             -          read_only
  *   passenger_root                                                  string             required   read_only
  *   pid_file                                                        string             -          read_only
diff --git a/src/agent/Core/Controller/Config.h b/src/agent/Core/Controller/Config.h
@@ -115,6 +115,7 @@ parseControllerBenchmarkMode(const StaticString &mode) {
  *   max_instances_per_app                               unsigned integer   -          read_only
  *   min_spare_clients                                   unsigned integer   -          default(0)
  *   multi_app                                           boolean            -          default(true),read_only
+ *   old_routing                                         boolean            -          default(false),read_only
  *   request_freelist_limit                              unsigned integer   -          default(1024)
  *   response_buffer_high_watermark                      unsigned integer   -          default(134217728)
  *   server_software                                     string             -          default("Phusion_Passenger/6.0.25")
@@ -138,6 +139,7 @@ class ControllerSchema: public ServerKit::HttpServerSchema {
 		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);
+		add("old_routing", BOOL_TYPE, OPTIONAL | READ_ONLY, false);
 		add("integration_mode", STRING_TYPE, OPTIONAL | READ_ONLY, DEFAULT_INTEGRATION_MODE);
 
 		add("user_switching", BOOL_TYPE, OPTIONAL, true);
@@ -349,6 +351,7 @@ class ControllerMainConfig {
 	bool userSwitching: 1;
 	bool defaultStickySessions: 1;
 	bool gracefulExit: 1;
+	bool oldRouting: 1;
 
 	/*******************/
 	/*******************/
@@ -366,7 +369,8 @@ class ControllerMainConfig {
 		  singleAppMode(!config["multi_app"].asBool()),
 		  userSwitching(config["user_switching"].asBool()),
 		  defaultStickySessions(config["default_sticky_sessions"].asBool()),
-		  gracefulExit(config["graceful_exit"].asBool())
+		  gracefulExit(config["graceful_exit"].asBool()),
+		  oldRouting(config["old_routing"].asBool())
 
 		  /*******************/
 	{
@@ -396,6 +400,7 @@ class ControllerMainConfig {
 		SWAP_BITFIELD(bool, userSwitching);
 		SWAP_BITFIELD(bool, defaultStickySessions);
 		SWAP_BITFIELD(bool, gracefulExit);
+		SWAP_BITFIELD(bool, oldRouting);
 
 		/*******************/
 
diff --git a/src/agent/Core/Controller/InitRequest.cpp b/src/agent/Core/Controller/InitRequest.cpp
@@ -206,7 +206,7 @@ Controller::fillPoolOptionsFromConfigCaches(Options &options,
 	options.statThrottleRate = mainConfig.statThrottleRate;
 	options.maxRequests = requestConfig->defaultMaxRequests;
 	options.stickySessionsCookieAttributes = requestConfig->defaultStickySessionsCookieAttributes;
-
+	options.oldRouting = mainConfig.oldRouting;
 	/******************************/
 }
 
diff --git a/src/agent/Core/OptionParser.h b/src/agent/Core/OptionParser.h
@@ -181,6 +181,8 @@ coreUsage() {
 	printf("                            Vary the turbocache by the cookie of the given name\n");
 	printf("      --disable-turbocaching\n");
 	printf("                            Disable turbocaching\n");
+	printf("      --old-routing\n");
+	printf("                            Revert to old routing algorithm\n");
 	printf("      --no-abort-websockets-on-process-shutdown\n");
 	printf("                            Do not abort WebSocket connections on process\n");
 	printf("                            shutdown or restart\n");
@@ -366,6 +368,9 @@ parseCoreOption(int argc, const char *argv[], int &i, Json::Value &updates) {
 	} else if (p.isFlag(argv[i], '\0', "--disable-turbocaching")) {
 		updates["turbocaching"] = false;
 		i++;
+	} else if (p.isFlag(argv[i], '\0', "--old-routing")) {
+		updates["old_routing"] = true;
+		i++;
 	} else if (p.isFlag(argv[i], '\0', "--no-abort-websockets-on-process-shutdown")) {
 		updates["default_abort_websockets_on_process_shutdown"] = false;
 		i++;
diff --git a/src/agent/Watchdog/Config.h b/src/agent/Watchdog/Config.h
@@ -144,6 +144,7 @@ using namespace std;
  *   max_instances_per_app                                                    unsigned integer   -          read_only
  *   max_pool_size                                                            unsigned integer   -          default(6)
  *   multi_app                                                                boolean            -          default(false),read_only
+ *   old_routing                                                              boolean            -          default(false),read_only
  *   passenger_root                                                           string             required   read_only
  *   pidfiles_to_delete_on_exit                                               array of strings   -          default([])
  *   pool_idle_time                                                           unsigned integer   -          default(300)
diff --git a/src/apache2_module/ConfigGeneral/AutoGeneratedDefinitions.cpp b/src/apache2_module/ConfigGeneral/AutoGeneratedDefinitions.cpp
@@ -332,6 +332,11 @@ extern "C" const command_rec passenger_commands[] = {
 		NULL,
 		RSRC_CONF | ACCESS_CONF,
 		"The Node.js command to use."),
+	AP_INIT_FLAG("PassengerOldRouting",
+		(FlagFunc) cmd_passenger_old_routing,
+		NULL,
+		RSRC_CONF,
+		"Whether to revert to old routing behaviour in Phusion Passenger(R)."),
 	AP_INIT_TAKE1("PassengerPoolIdleTime",
 		(Take1Func) cmd_passenger_pool_idle_time,
 		NULL,
diff --git a/src/apache2_module/ConfigGeneral/AutoGeneratedManifestDefaultsInitialization.cpp b/src/apache2_module/ConfigGeneral/AutoGeneratedManifestDefaultsInitialization.cpp
@@ -108,6 +108,11 @@ ConfigManifestGenerator::autoGenerated_setGlobalConfigDefaults() {
 		"PassengerMaxPoolSize",
 		DEFAULT_MAX_POOL_SIZE);
 
+	addOptionsContainerStaticDefaultBool(
+		globalConfigContainer,
+		"PassengerOldRouting",
+		false);
+
 	addOptionsContainerStaticDefaultInt(
 		globalConfigContainer,
 		"PassengerPoolIdleTime",
diff --git a/src/apache2_module/ConfigGeneral/AutoGeneratedSetterFuncs.cpp b/src/apache2_module/ConfigGeneral/AutoGeneratedSetterFuncs.cpp
@@ -715,6 +715,21 @@ cmd_passenger_nodejs(cmd_parms *cmd, void *pcfg, const char *arg) {
 	return NULL;
 }
 
+static const char *
+cmd_passenger_old_routing(cmd_parms *cmd, void *pcfg, const char *arg) {
+	const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+	if (err != NULL) {
+		ap_log_perror(APLOG_MARK, APLOG_STARTUP, 0, cmd->temp_pool,
+			"WARNING: %s", err);
+	}
+
+	serverConfig.oldRoutingSourceFile = cmd->directive->filename;
+	serverConfig.oldRoutingSourceLine = cmd->directive->line_num;
+	serverConfig.oldRoutingExplicitlySet = true;
+	serverConfig.oldRouting = arg != NULL;
+	return NULL;
+}
+
 static const char *
 cmd_passenger_pool_idle_time(cmd_parms *cmd, void *pcfg, const char *arg) {
 	const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
diff --git a/src/apache2_module/Hooks.cpp b/src/apache2_module/Hooks.cpp
@@ -1380,6 +1380,7 @@ class Hooks {
 		config["response_buffer_high_watermark"] = serverConfig.responseBufferHighWatermark;
 		config["stat_throttle_rate"] = serverConfig.statThrottleRate;
 		config["turbocaching"] = serverConfig.turbocaching;
+		config["old_routing"] = serverConfig.oldRouting;
 		config["prestart_urls"] = strsetToJson(serverConfig.prestartURLs);
 		config["admin_panel_url"] = nonEmptyString(serverConfig.adminPanelUrl);
 		config["admin_panel_auth_type"] = nonEmptyString(serverConfig.adminPanelAuthType);
diff --git a/src/apache2_module/ServerConfig/AutoGeneratedManifestGeneration.cpp b/src/apache2_module/ServerConfig/AutoGeneratedManifestGeneration.cpp
@@ -244,6 +244,15 @@ ConfigManifestGenerator::autoGenerated_generateConfigManifestForServerConfig() {
 			serverConfig.maxPoolSizeSourceLine);
 		hierarchyMember["value"] = serverConfig.maxPoolSize;
 	}
+	if (serverConfig.oldRoutingExplicitlySet) {
+		Json::Value &optionContainer = findOrCreateOptionContainer(globalOptionsContainer,
+			"PassengerOldRouting",
+			sizeof("PassengerOldRouting") - 1);
+		Json::Value &hierarchyMember = addOptionContainerHierarchyMember(optionContainer,
+			serverConfig.oldRoutingSourceFile,
+			serverConfig.oldRoutingSourceLine);
+		hierarchyMember["value"] = serverConfig.oldRouting == Apache2Module::ENABLED;
+	}
 	if (serverConfig.poolIdleTimeExplicitlySet) {
 		Json::Value &optionContainer = findOrCreateOptionContainer(globalOptionsContainer,
 			"PassengerPoolIdleTime",
diff --git a/src/apache2_module/ServerConfig/AutoGeneratedStruct.h b/src/apache2_module/ServerConfig/AutoGeneratedStruct.h
@@ -73,6 +73,11 @@ struct AutoGeneratedServerConfig {
 	 */
 	bool disableSecurityUpdateCheck;
 
+	/*
+	 * Whether to revert to old routing behaviour in Phusion Passenger(R).
+	 */
+	bool oldRouting;
+
 	/*
 	 * Whether to show the Phusion Passenger(R) version number in the X-Powered-By header.
 	 */
@@ -212,6 +217,7 @@ struct AutoGeneratedServerConfig {
 	StaticString disableAnonymousTelemetrySourceFile;
 	StaticString disableLogPrefixSourceFile;
 	StaticString disableSecurityUpdateCheckSourceFile;
+	StaticString oldRoutingSourceFile;
 	StaticString showVersionInHeaderSourceFile;
 	StaticString turbocachingSourceFile;
 	StaticString userSwitchingSourceFile;
@@ -243,6 +249,7 @@ struct AutoGeneratedServerConfig {
 	unsigned int disableAnonymousTelemetrySourceLine;
 	unsigned int disableLogPrefixSourceLine;
 	unsigned int disableSecurityUpdateCheckSourceLine;
+	unsigned int oldRoutingSourceLine;
 	unsigned int showVersionInHeaderSourceLine;
 	unsigned int turbocachingSourceLine;
 	unsigned int userSwitchingSourceLine;
@@ -274,6 +281,7 @@ struct AutoGeneratedServerConfig {
 	bool disableAnonymousTelemetryExplicitlySet: 1;
 	bool disableLogPrefixExplicitlySet: 1;
 	bool disableSecurityUpdateCheckExplicitlySet: 1;
+	bool oldRoutingExplicitlySet: 1;
 	bool showVersionInHeaderExplicitlySet: 1;
 	bool turbocachingExplicitlySet: 1;
 	bool userSwitchingExplicitlySet: 1;
@@ -307,6 +315,7 @@ struct AutoGeneratedServerConfig {
 		disableAnonymousTelemetry = false;
 		disableLogPrefix = false;
 		disableSecurityUpdateCheck = false;
+		oldRouting = false;
 		showVersionInHeader = true;
 		turbocaching = true;
 		userSwitching = true;
@@ -368,6 +377,7 @@ struct AutoGeneratedServerConfig {
 		disableAnonymousTelemetrySourceLine = 0;
 		disableLogPrefixSourceLine = 0;
 		disableSecurityUpdateCheckSourceLine = 0;
+		oldRoutingSourceLine = 0;
 		showVersionInHeaderSourceLine = 0;
 		turbocachingSourceLine = 0;
 		userSwitchingSourceLine = 0;
@@ -399,6 +409,7 @@ struct AutoGeneratedServerConfig {
 		disableAnonymousTelemetryExplicitlySet = false;
 		disableLogPrefixExplicitlySet = false;
 		disableSecurityUpdateCheckExplicitlySet = false;
+		oldRoutingExplicitlySet = false;
 		showVersionInHeaderExplicitlySet = false;
 		turbocachingExplicitlySet = false;
 		userSwitchingExplicitlySet = false;
diff --git a/src/nginx_module/ConfigGeneral/AutoGeneratedDefinitions.c b/src/nginx_module/ConfigGeneral/AutoGeneratedDefinitions.c
@@ -190,6 +190,14 @@
     offsetof(passenger_main_conf_t, autogenerated.turbocaching),
     NULL
 },
+{
+    ngx_string("passenger_old_routing"),
+    NGX_HTTP_MAIN_CONF | NGX_CONF_FLAG,
+    passenger_conf_set_old_routing,
+    NGX_HTTP_MAIN_CONF_OFFSET,
+    offsetof(passenger_main_conf_t, autogenerated.old_routing),
+    NULL
+},
 {
     ngx_string("passenger_user_switching"),
     NGX_HTTP_MAIN_CONF | NGX_CONF_FLAG,
diff --git a/src/nginx_module/ConfigGeneral/AutoGeneratedManifestDefaultsInitialization.c b/src/nginx_module/ConfigGeneral/AutoGeneratedManifestDefaultsInitialization.c
@@ -119,6 +119,12 @@ set_manifest_autogenerated_global_conf_defaults(manifest_gen_ctx_t *ctx) {
         sizeof("passenger_turbocaching") - 1,
         1);
 
+    add_manifest_options_container_static_default_bool(ctx,
+        ctx->global_config_container,
+        "passenger_old_routing",
+        sizeof("passenger_old_routing") - 1,
+        0);
+
     add_manifest_options_container_static_default_bool(ctx,
         ctx->global_config_container,
         "passenger_user_switching",
diff --git a/src/nginx_module/ConfigGeneral/AutoGeneratedSetterFuncs.c b/src/nginx_module/ConfigGeneral/AutoGeneratedSetterFuncs.c
diff --git a/src/nginx_module/MainConfig/AutoGeneratedCreateFunction.c b/src/nginx_module/MainConfig/AutoGeneratedCreateFunction.c
diff --git a/src/nginx_module/MainConfig/AutoGeneratedManifestGeneration.c b/src/nginx_module/MainConfig/AutoGeneratedManifestGeneration.c
diff --git a/src/nginx_module/MainConfig/AutoGeneratedStruct.h b/src/nginx_module/MainConfig/AutoGeneratedStruct.h
diff --git a/src/nginx_module/ngx_http_passenger_module.c b/src/nginx_module/ngx_http_passenger_module.c
diff --git a/src/ruby_supportlib/phusion_passenger/apache2/config_options.rb b/src/ruby_supportlib/phusion_passenger/apache2/config_options.rb
diff --git a/src/ruby_supportlib/phusion_passenger/nginx/config_options.rb b/src/ruby_supportlib/phusion_passenger/nginx/config_options.rb
diff --git a/src/ruby_supportlib/phusion_passenger/standalone/config_options_list.rb b/src/ruby_supportlib/phusion_passenger/standalone/config_options_list.rb
diff --git a/src/ruby_supportlib/phusion_passenger/standalone/start_command/builtin_engine.rb b/src/ruby_supportlib/phusion_passenger/standalone/start_command/builtin_engine.rb

Original file line number	Diff line number	Diff line change
`@@ -136,6 +136,7 @@ Group::Group(Pool *_pool, const Options &_options)`
`136`	`136`	`}`
`137`	`137`
`138`	`138`	`detachedProcessesCheckerActive = false;`
	`139`	`+ oldRouting = _options.oldRouting;`
`139`	`140`	`}`
`140`	`141`
`141`	`142`	`Group::~Group() {`
Original file line number	Diff line number	Diff line change
`@@ -206,7 +206,7 @@ Controller::fillPoolOptionsFromConfigCaches(Options &options,`
`206`	`206`	`options.statThrottleRate = mainConfig.statThrottleRate;`
`207`	`207`	`options.maxRequests = requestConfig->defaultMaxRequests;`
`208`	`208`	`options.stickySessionsCookieAttributes = requestConfig->defaultStickySessionsCookieAttributes;`
`209`		`-`
	`209`	`+ options.oldRouting = mainConfig.oldRouting;`
`210`	`210`	`/******************************/`
`211`	`211`	`}`
`212`	`212`