-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Modify bin/apisix to support the SO_REUSEPORT #1085
Conversation
…generated nginx.conf file can open the reuseport configuration Judgment logic: 1. If it is a mac system, do not open reuseport 2. If it is a linux system and the kernel version is higher than 3.9.0, then turn on the reuserport switch, and the reuseport option will be added when generating nginx.conf> configuration
bin/apisix
Outdated
{% end %} | ||
|
||
{% else %} {% --if enable_reuseport %} | ||
|
||
{% for _, port in ipairs(stream_proxy.tcp or {}) do %} | ||
listen {*port*}; |
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.
listen {*port*} {% if enable_reuseport then %} reuseport {% end %};
This style is simpler.
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.
please add test cases.
Test cases to be added after pull request # 1063 apache#1063
Strictly speaking, detect if a feature is available or not via kernel version is not reliable. Linux distributions may backport kernel patch to an old release (search for "SO_REUSEPORT backport for linux distribution"), not even mentioning self-patched kernel. I think a better solution is to allow the users to configure it, since they know better about their servers than us. Maybe we can enable this feature by default, because most of the servers have kernel newer than 3.9. |
Considering that the kernel version of the current production environment basically supports reuseport, I think it is more reasonable to open the REUSEPORT configuration file by default and supporting configuration it. |
I remember that there are two main pits in reuseport, one is the kernel version, and the other is the support of http2 and http3. |
this way is simpler. thx @spacewander @Miss-you Expect your new commit ^_^ |
…uction environment basically supports reuseport, I think it is more reasonable to open the REUSEPORT configuration file by default and supporting configuration it. So, it's a better solution is to allow the users to configure the 'enable_reuseport'.
@membphis @spacewander |
bin/apisix
Outdated
@@ -507,6 +507,7 @@ local function init() | |||
with_module_status = with_module_status, | |||
node_ssl_listen = 9443, -- default value | |||
error_log = {level = "warn"}, | |||
enable_reuseport = true, -- default true |
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.
the default value should be set in config.yaml
, not here.
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
bin/apisix
Outdated
if sys_conf["enable_reuseport"] == false then | ||
enable_reuseport = false | ||
end | ||
|
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.
we can use the enable_reuseport
in config.yaml
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
waiting your test case ^_^ |
update apache/incubator-apisix
conf/config.yaml
Outdated
@@ -21,7 +21,7 @@ apisix: | |||
enable_admin_cors: true # Admin API support CORS response headers. | |||
enable_debug: false | |||
enable_dev_mode: false # Sets nginx worker_processes to 1 if set to true | |||
enable_reuseport: true # Enable nginx SO_REUSEPORT switch if set to true. | |||
enable_reuseport: false # Enable nginx SO_REUSEPORT switch if set to true. |
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.
default value true
is better
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.
setting enable_reuseport false to verify that test cases are working.
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.
For testing, you can write a separate script to modify config.yaml
.
2b61386
to
56e1d97
Compare
5a7ec3d
to
62dc89d
Compare
…_runner.sh 1.fix issue 'can not find openresty' 2.remove function 'check_result' 3.fix .gitignore
ok, we can merge it to |
NOTE: Please read the Contributing.md guidelines before submitting your patch:
https://github.com/apache/incubator-apisix/blob/master/Contributing.md#how-to-add-a-new-feature-or-change-an-existing-one
Summary
Modify bin/apisix to increase the capacity so that the generated nginx.conf file can open the reuseport configuration
Judgment logic:
1. If it is a mac system, do not open reuseport
2. If it is a linux system and the kernel version is higher than 3.9.0, then turn on the reuserport switch, and the reuseport option will be added when generating nginx.conf> configuration
Full changelog
bin/apisix
Issues resolved
Fix #342