kingluo commented on code in PR #9016:
URL: https://github.com/apache/apisix/pull/9016#discussion_r1127525269
##########
apisix/upstream.lua:
##########
@@ -115,8 +122,11 @@ local function create_checker(upstream)
end
if healthcheck_parent.checker then
Review Comment:
@monkeyDluffy6017 You're wrong. Sometimes reading code is not enough, and
you need to debug it to guarantee your point.
https://github.com/apache/apisix/blob/d8fd3676ede5c07ccce4215f3dfbd823939dcf36/apisix/core/config_etcd.lua#L472
Set up a breakpoint:
```bash
cat > /usr/local/apisix/plugin_inspect_hooks.lua <<EOF
dbg.set_hook("apisix/core/config_etcd.lua", 472, nil, function(info)
core.log.warn("res: ", tostring(info.vals.res))
return false
end)
EOF
```
error.log:
```
2023/03/07 16:19:33 [warn] 2472696#2472696: *110 [lua] init.lua:68:
setup_hooks(): set hooks: err: true, hooks:
["apisix/core/config_etcd.lua#472"], context: ngx.timer
```
Create a test upstream:
```bash
update_upstream() {
curl http://127.0.0.1:9180/apisix/admin/upstreams/1 \
-H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
"nodes":[
{
"host":"httpbin.org",
"port":80,
"weight":1
},
{
"host":"postman-echo.com",
"port":80,
"weight":1
}
]
}'
}
update_upstream()
```
Override:
```bash
update_upstream()
```
error.log:
```
2023/03/07 16:20:09 [warn] 2472696#2472696: *35 [lua] [string "local dbg =
require("apisix.inspect.dbg")..."]:256: res: table: 0x7f5c62fede28, context:
ngx.timer
```
Override again:
```bash
update_upstream()
```
error.log:
```
2023/03/07 16:20:25 [warn] 2472696#2472696: *35 [lua] [string "local dbg =
require("apisix.inspect.dbg")..."]:256: res: table: 0x7f5c680bbe88, context:
ngx.timer
```
Note that the table is different: `old: 0x7f5c62fede28, new: 0x7f5c680bbe88`.
The parent of an upstream is itself,
https://github.com/apache/apisix/blob/d8fd3676ede5c07ccce4215f3dfbd823939dcf36/apisix/upstream.lua#L564
https://github.com/apache/apisix/blob/d8fd3676ede5c07ccce4215f3dfbd823939dcf36/apisix/upstream.lua#L510
So, each time the upstream gets updated, the parent is new, so it's
impossible to enter into cases where the parent is the same but with a
different checker, except this parallel create_checker bug, and since we do the
bugfix, then no need to keep such code anymore. It works for all cases,
including route with inline upstream or service with inline upstream.
@spacewander @moonming @membphis I think we should remove such obsolete code
to make our code base lint.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
