Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(etcd): reuse cli and enable keepalive #9420

Merged
merged 4 commits into from
May 17, 2023
Merged

fix(etcd): reuse cli and enable keepalive #9420

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c0b4b09
fix(etcd): reuse cli and enable keepalive
kingluo May 6, 2023
5668c32
fix PR
kingluo May 11, 2023
7413606
fix PR
kingluo May 11, 2023
ce4d835
revert a test case
kingluo May 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apisix/cli/snippet.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ upstream apisix_conf_backend {
local conf_server = require("apisix.conf_server")
conf_server.balancer()
}
keepalive 320;
keepalive_requests 1000;
keepalive_timeout 60s;
}

{% if trusted_ca_cert then %}
Expand Down
4 changes: 1 addition & 3 deletions apisix/core/etcd.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,7 @@ do
return nil, nil, err
end

if tmp_etcd_cli.use_grpc then
etcd_cli = tmp_etcd_cli
end
etcd_cli = tmp_etcd_cli

return tmp_etcd_cli, prefix
end
Expand Down
47 changes: 5 additions & 42 deletions t/core/config_etcd.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,44 +233,7 @@ passed



=== TEST 7: ensure only one auth request per subsystem for all the etcd sync
--- yaml_config
apisix:
node_listen: 1984
deployment:
role: traditional
role_traditional:
config_provider: etcd
etcd:
host:
- "http://127.0.0.1:1980" -- fake server port
timeout: 1
user: root # root username for etcd
password: 5tHkHhYkjr6cQY # root password for etcd
--- extra_init_by_lua
local health_check = require("resty.etcd.health_check")
health_check.get_target_status = function()
return true
end
--- config
location /t {
content_by_lua_block {
ngx.sleep(0.5)
}
}
--- request
GET /t
--- grep_error_log eval
qr/etcd auth failed/
--- grep_error_log_out
etcd auth failed
monkeyDluffy6017 marked this conversation as resolved.
Show resolved Hide resolved
etcd auth failed
etcd auth failed
etcd auth failed
monkeyDluffy6017 marked this conversation as resolved.
Show resolved Hide resolved



=== TEST 8: ensure add prefix automatically for _M.getkey
=== TEST 7: ensure add prefix automatically for _M.getkey
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -301,7 +264,7 @@ passed



=== TEST 9: Test ETCD health check mode switch during APISIX startup
=== TEST 8: Test ETCD health check mode switch during APISIX startup
--- config
location /t {
content_by_lua_block {
Expand All @@ -320,7 +283,7 @@ qr/healthy check use round robin



=== TEST 10: last_err can be nil when the reconnection is successful
=== TEST 9: last_err can be nil when the reconnection is successful
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -350,7 +313,7 @@ passed



=== TEST 11: reloaded data may be in res.body.node (special kvs structure)
=== TEST 10: reloaded data may be in res.body.node (special kvs structure)
--- yaml_config
deployment:
role: traditional
Expand Down Expand Up @@ -397,7 +360,7 @@ qr/readdir key: fake res: \{("value":"bar","key":"foo"|"key":"foo","value":"bar"



=== TEST 12: reloaded data may be in res.body.node (admin_api_version is v2)
=== TEST 11: reloaded data may be in res.body.node (admin_api_version is v2)
--- yaml_config
deployment:
role: traditional
Expand Down
32 changes: 0 additions & 32 deletions t/core/etcd.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,35 +401,3 @@ qr/init_by_lua:\d+: \S+/
init_by_lua:12: ab
init_by_lua:19: 200
init_by_lua:26: 404



=== TEST 8: error handling in server_version
--- yaml_config
deployment:
role: traditional
role_traditional:
config_provider: etcd
etcd:
host:
- "http://127.0.0.1:2379"
prefix: "/apisix"
--- config
location /t {
content_by_lua_block {
local etcd_lib = require("resty.etcd")
-- the mock won't take effect when using gRPC because the connection will be cached
etcd_lib.new = function()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hijacking resty.etcd.new does not work after the cli is created once, because it's reused afterwards.

return nil, "ouch"
end
local etcd = require("apisix.core.etcd")
local res, err = etcd.server_version()
ngx.say(err)
}
}
--- request
GET /t
--- response_body
ouch
--- error_log
failed to get server_info from etcd