Allow opting out of new routing algorithm via a flag (#2590)

* env var based routing selection

* add a flag for using old routing

* move oldRouting into pool context from group

* fix tests

Author: CamJN

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

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,

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) %>

src/agent/Core/ApplicationPool/Context.h

@@ -61,16 +61,19 @@ struct Context {
 	boost::object_pool<Process> processObjectPool;
 	mutable boost::mutex agentConfigSyncher;
 
+	// Whether to use the old routing algorithm
+	bool oldRouting;
 
 	/****** Dependencies ******/
 
 	SpawningKit::FactoryPtr spawningKitFactory;
 	Json::Value agentConfig;
 
 
-	Context()
+	Context(bool _oldRouting)
 		: sessionObjectPool(64, 1024),
-		  processObjectPool(4, 64)
+		  processObjectPool(4, 64),
+		  oldRouting(_oldRouting)
 		{ }
 
 	void finalize() {

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,7 +200,6 @@ class Group: public boost::enable_shared_from_this<Group> {
 	Callback shutdownCallback;
 	GroupPtr selfPointer;
 
-
 	/****** Initialization and shutdown ******/
 
 	static ApiKey generateApiKey(const Pool *pool);
@@ -233,9 +230,13 @@ class Group: public boost::enable_shared_from_this<Group> {
 	/****** Process list management ******/
 
 	Process *findProcessWithStickySessionId(unsigned int id) const;
+	Process *findProcessWithStickySessionIdOrLowestBusyness(unsigned int id) const;
+	Process *findProcessWithLowestBusyness(const ProcessList &processes) const;
+	Process *findEnabledProcessWithLowestBusyness() const;
 	Process *findBestProcessPreferringStickySessionId(unsigned int id) const;
 	Process *findBestProcess(const ProcessList &processes) const;
 	Process *findBestEnabledProcess() const;
+	bool useNewRouting() const;
 
 	void addProcessToList(const ProcessPtr &process, ProcessList &destination);
 	void removeProcessFromList(const ProcessPtr &process, ProcessList &source);

src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp

@@ -63,6 +63,73 @@ Group::findProcessWithStickySessionId(unsigned int id) const {
 	return NULL;
 }
 
+Process *
+Group::findProcessWithStickySessionIdOrLowestBusyness(unsigned int id) const {
+	int leastBusyProcessIndex = -1;
+	int lowestBusyness = 0;
+	unsigned int i, size = enabledProcessBusynessLevels.size();
+	const int *enabledProcessBusynessLevels = &this->enabledProcessBusynessLevels[0];
+	for (i = 0; i < size; i++) {
+		Process *process = enabledProcesses[i].get();
+		if (process->getStickySessionId() == id) {
+			return process;
+		} else if (leastBusyProcessIndex == -1 || enabledProcessBusynessLevels[i] < lowestBusyness) {
+			leastBusyProcessIndex = i;
+			lowestBusyness = enabledProcessBusynessLevels[i];
+		}
+	}
+
+	if (leastBusyProcessIndex == -1) {
+		return NULL;
+	} else {
+		return enabledProcesses[leastBusyProcessIndex].get();
+	}
+}
+
+Process *
+Group::findProcessWithLowestBusyness(const ProcessList &processes) const {
+	if (processes.empty()) {
+		return NULL;
+	}
+
+	int lowestBusyness = -1;
+	Process *leastBusyProcess = NULL;
+	ProcessList::const_iterator it;
+	ProcessList::const_iterator end = processes.end();
+	for (it = processes.begin(); it != end; it++) {
+		Process *process = (*it).get();
+		int busyness = process->busyness();
+		if (lowestBusyness == -1 || lowestBusyness > busyness) {
+			lowestBusyness = busyness;
+			leastBusyProcess = process;
+		}
+	}
+	return leastBusyProcess;
+}
+
+/**
+ * Cache-optimized version of findProcessWithLowestBusyness() for the common case.
+ */
+Process *
+Group::findEnabledProcessWithLowestBusyness() const {
+	if (enabledProcesses.empty()) {
+		return NULL;
+	}
+
+	int leastBusyProcessIndex = -1;
+	int lowestBusyness = 0;
+	unsigned int i, size = enabledProcessBusynessLevels.size();
+	const int *enabledProcessBusynessLevels = &this->enabledProcessBusynessLevels[0];
+
+	for (i = 0; i < size; i++) {
+		if (leastBusyProcessIndex == -1 || enabledProcessBusynessLevels[i] < lowestBusyness) {
+			leastBusyProcessIndex = i;
+			lowestBusyness = enabledProcessBusynessLevels[i];
+		}
+	}
+	return enabledProcesses[leastBusyProcessIndex].get();
+}
+
 /**
  * Return the process with the given sticky session ID if it exists.
  * If not, then find the "best" enabled process to route a request to,

src/agent/Core/ApplicationPool/Group/SessionManagement.cpp

@@ -30,7 +30,6 @@
 	#include <Core/ApplicationPool/Pool.h>
 #endif
 #include <Core/ApplicationPool/Group.h>
-#include <cassert>
 
 /*************************************************************************
  *
@@ -63,18 +62,26 @@ using namespace boost;
  */
 Group::RouteResult
 Group::route(const Options &options) const {
+	Process *process = nullptr;
 	if (OXT_LIKELY(enabledCount > 0)) {
 		if (options.stickySessionId == 0) {
-			Process *process = findBestProcess(enabledProcesses);
+			if (OXT_LIKELY(useNewRouting())) {
+				process = findBestProcess(enabledProcesses);
+            } else {
+				process = findEnabledProcessWithLowestBusyness();
+			}
 			if (process != nullptr) {
 				assert(process->canBeRoutedTo());
 				return RouteResult(process);
 			} else {
 				return RouteResult(NULL, true);
 			}
 		} else {
-			Process *process = findBestProcessPreferringStickySessionId(
-				options.stickySessionId);
+			if (OXT_LIKELY(useNewRouting())) {
+				process = findBestProcessPreferringStickySessionId(options.stickySessionId);
+			}else{
+				process = findProcessWithStickySessionIdOrLowestBusyness(options.stickySessionId);
+			}
 			if (process != nullptr) {
 				if (process->canBeRoutedTo()) {
 					return RouteResult(process);
@@ -86,7 +93,11 @@ Group::route(const Options &options) const {
 			}
 		}
 	} else {
-		Process *process = findBestProcess(disablingProcesses);
+		if (OXT_LIKELY(useNewRouting())) {
+			process = findBestProcess(disablingProcesses);
+		} else {
+			process = findProcessWithLowestBusyness(disablingProcesses);
+		}
 		if (process != nullptr) {
 			assert(process->canBeRoutedTo());
 			return RouteResult(process);
@@ -313,7 +324,12 @@ Group::get(const Options &newOptions, const GetCallback &callback,
 		assert(m_spawning || restarting() || poolAtFullCapacity());
 
 		if (disablingCount > 0 && !restarting()) {
-			Process *process = findBestProcess(disablingProcesses);
+			Process *process = nullptr;
+			if (OXT_LIKELY(useNewRouting())) {
+				process = findBestProcess(disablingProcesses);
+			} else {
+				process = findProcessWithLowestBusyness(disablingProcesses);
+			}
 			if (process != nullptr && !process->isTotallyBusy()) {
 				return newSession(process, newOptions.currentTime);
 			}
@@ -341,6 +357,10 @@ Group::get(const Options &newOptions, const GetCallback &callback,
 	}
 }
 
+bool
+Group::useNewRouting() const {
+	return !pool->context->oldRouting;
+}
 
 } // namespace ApplicationPool2
 } // namespace Passenger

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);

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

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);
 
 		/*******************/