spacewander commented on code in PR #8130:
URL: https://github.com/apache/apisix/pull/8130#discussion_r1001420302
##########
apisix/plugins/ai.lua:
##########
@@ -100,33 +108,120 @@ local function gen_get_cache_key_func(route_flags)
end
+local function ai_upstream()
+ core.log.info("enable sample upstream")
+end
+
+
+local pool_opt = { pool_size = 320 }
+local function ai_balancer_run(route)
+ local server = route.value.upstream.nodes[1]
+ if enable_keepalive then
+ local ok, err = balancer.set_current_peer(server.host, server.port or
80, pool_opt)
+ if not ok then
+ core.log.error("failed to set server peer [", server.host, ":",
+ server.port, "] err: ", err)
+ return ok, err
+ end
+ balancer.enable_keepalive(60, 1000)
Review Comment:
Why don't we use the keepalive configured in the yaml?
##########
apisix/plugins/ai.lua:
##########
@@ -138,6 +233,27 @@ local function routes_analyze(routes)
router.router_http.match = orig_router_match
end
end
+
+ if not route_flags["service"]
+ and not route_flags["service_id"]
+ and not route_flags["upstream_id"]
+ and not route_flags["enable_websocket"]
+ and not route_flags["plugins"]
+ and not route_up_flags["has_domain"]
+ and route_up_flags["pass_host"]
+ and route_up_flags["scheme"]
+ and not route_up_flags["checks"]
+ and not route_up_flags["retries"]
+ and not route_up_flags["timeout"]
+ and not route_up_flags["timeout"]
+ and not route_up_flags["keepalive"] then
Review Comment:
Is it possible to use allow list instead of a pile of deny list?
##########
apisix/plugins/ai.lua:
##########
@@ -100,33 +108,120 @@ local function gen_get_cache_key_func(route_flags)
end
+local function ai_upstream()
+ core.log.info("enable sample upstream")
+end
+
+
+local pool_opt = { pool_size = 320 }
+local function ai_balancer_run(route)
+ local server = route.value.upstream.nodes[1]
+ if enable_keepalive then
+ local ok, err = balancer.set_current_peer(server.host, server.port or
80, pool_opt)
+ if not ok then
+ core.log.error("failed to set server peer [", server.host, ":",
+ server.port, "] err: ", err)
+ return ok, err
+ end
+ balancer.enable_keepalive(60, 1000)
+ else
+ balancer.set_current_peer(server.host, server.port or 80)
+ end
Review Comment:
We can move:
```
if enable_keepalive then
```
to here?
Like:
```
if enable_keepalive then
balancer.enable_keepalive(60, 1000)
```
##########
apisix/plugins/ai.lua:
##########
@@ -138,6 +233,27 @@ local function routes_analyze(routes)
router.router_http.match = orig_router_match
end
end
+
+ if not route_flags["service"]
+ and not route_flags["service_id"]
+ and not route_flags["upstream_id"]
+ and not route_flags["enable_websocket"]
+ and not route_flags["plugins"]
+ and not route_up_flags["has_domain"]
+ and route_up_flags["pass_host"]
+ and route_up_flags["scheme"]
+ and not route_up_flags["checks"]
+ and not route_up_flags["retries"]
+ and not route_up_flags["timeout"]
+ and not route_up_flags["timeout"]
Review Comment:
Repeated condition?
--
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]
