Yiyiyimu commented on a change in pull request #4191:
URL: https://github.com/apache/apisix/pull/4191#discussion_r645176342
##########
File path: apisix/core/config_etcd.lua
##########
@@ -529,6 +531,17 @@ local function _automatic_fetch(premature, self)
return
end
+ if not health_check.conf then
Review comment:
In lua-resty-etcd
https://github.com/api7/lua-resty-etcd/blob/1fe01f77dda252f2d95eed0a7635f3479097b8ed/lib/resty/etcd/health_check.lua#L70-L83
##########
File path: apisix/core/config_etcd.lua
##########
@@ -529,6 +531,17 @@ local function _automatic_fetch(premature, self)
return
end
+ if not health_check.conf then
Review comment:
In lua-resty-etcd
https://github.com/api7/lua-resty-etcd/blob/1fe01f77dda252f2d95eed0a7635f3479097b8ed/lib/resty/etcd/health_check.lua#L70-L83
And I think I didn't configure it correctly but failed to find the reason.
The health check would renew its value in shared dict for every loop in
`automatic_fetch`, so etcd health check would keep refreshing the tainted node
status but not directly jump over it. Removing the error check imported in this
PR still got the same problem.
##########
File path: apisix/core/config_etcd.lua
##########
@@ -545,6 +558,33 @@ local function _automatic_fetch(premature, self)
local ok, err = sync_data(self)
if err then
+ if string.find(err, "connection refused")
+ or string.find(err, "Service Unavailable") then
+ local etcd_cli, err = get_etcd()
Review comment:
Got it. Working on it
##########
File path: apisix/core/config_etcd.lua
##########
@@ -545,6 +558,33 @@ local function _automatic_fetch(premature, self)
local ok, err = sync_data(self)
if err then
+ if string.find(err, "connection refused")
+ or string.find(err, "Service Unavailable") then
+ local etcd_cli, err = get_etcd()
+ if not etcd_cli then
+ error("all etcd endpoints are unhealthy: " .. err)
+ end
+ self.etcd_cli = etcd_cli
+ log.warn("changed to new etcd endpoint")
+ end
+ if string.find(err, "has no healthy etcd endpoint available")
then
Review comment:
Fixed thanks!
##########
File path: apisix/core/config_etcd.lua
##########
@@ -545,6 +558,33 @@ local function _automatic_fetch(premature, self)
local ok, err = sync_data(self)
if err then
+ if string.find(err, "connection refused")
+ or string.find(err, "Service Unavailable") then
+ local etcd_cli, err = get_etcd()
Review comment:
Changed to forcibly get new etcd client
##########
File path: apisix/core/config_etcd.lua
##########
@@ -545,7 +558,34 @@ local function _automatic_fetch(premature, self)
local ok, err = sync_data(self)
if err then
- if err ~= "timeout" and err ~= "Key not found"
+ if string.find(err, "connection refused")
+ or string.find(err, "Service Unavailable") then
+ log.warn(err, ", ", tostring(self))
Review comment:
Mainly to fulfill the needs of config-etcd test, and I think it's also
good to inform users that etcd can not be connected
https://github.com/apache/apisix/blob/86e168e711713531eeafdd0a75682b150e0d0109/t/core/config_etcd.t#L28-L49
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
