-
Notifications
You must be signed in to change notification settings - Fork 548
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
Allow opting out of new routing algorithm via a flag #2590
base: stable-6.0
Are you sure you want to change the base?
Conversation
90e4797
to
0a11528
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly good, just requesting some minor config handling changes.
@@ -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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since on the web server level this is a global config anyway, you can turn this into a pool-level flag (i.e., a boolean in ApplicationPool2::Context) instead of a Group-level flag. That simplifies some code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
484f843
to
eaf3953
Compare
Due to #2579 we decided to make the new routing optional. One option was to only use the new routing when rolling restarting, but there isn't a good way to check if that is going on as far as I can tell. So in order to do this quickly, I am just adding a flag. I don't expect this to be a very commonly needed setting.
If there is a better way to check for rolling restarts, I can easily change the implementation of
useNewRouting
.