You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "ranxuxin (via GitHub)" <gi...@apache.org> on 2023/04/19 03:07:18 UTC
[GitHub] [apisix] ranxuxin opened a new pull request, #9334: feature:increment modifying routes performance
ranxuxin opened a new pull request, #9334:
URL: https://github.com/apache/apisix/pull/9334
refer to #9259
pressure test:
environment: centos 7 8core 32G mem
wrk -c1000 -d60s -t8 --latency --timeout=15s http://www.aaabbb1122.com:9080/123
patched version:
elapsed time to change a route:
original version:
elapsed time to change a route:
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9334: feat: incremental route radixtree update
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9334:
URL: https://github.com/apache/apisix/pull/9334#discussion_r1222422756
##########
apisix/router.lua:
##########
@@ -22,31 +22,297 @@ local plugin_checker = require("apisix.plugin").plugin_checker
local str_lower = string.lower
local error = error
local ipairs = ipairs
-
+local sub_str = string.sub
+local table = require("apisix.core.table")
+local json = require("apisix.core.json")
+local router_util = require("apisix.utils.router")
+local tab_insert = table.insert
local _M = {version = 0.3}
+local function empty_func() end
-local function filter(route)
- route.orig_modifiedIndex = route.modifiedIndex
- route.update_count = 0
+local function push_host_router(route, host_routes, only_uri_routes)
Review Comment:
export and reuse push_host_router() in radixtree_host_uri.lua.
##########
apisix/router.lua:
##########
@@ -70,7 +336,6 @@ local function attach_http_router_common_methods(http_router)
end
end
-
Review Comment:
revert blank line delete.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9334: feat: incremental route radixtree update
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9334:
URL: https://github.com/apache/apisix/pull/9334#discussion_r1196102608
##########
apisix/router.lua:
##########
@@ -22,34 +22,147 @@ local plugin_checker = require("apisix.plugin").plugin_checker
local str_lower = string.lower
local error = error
local ipairs = ipairs
-
+local sub_str = string.sub
+local table = require("apisix.core.table")
+local json = require("apisix.core.json")
local _M = {version = 0.3}
+local function short_key(self, str)
+ return sub_str(str, #self.key + 2)
+end
-local function filter(route)
+local function filter(route, pre_route_obj, size)
route.orig_modifiedIndex = route.modifiedIndex
- route.update_count = 0
+ route.update_count = 0
route.has_domain = false
- if not route.value then
+ if route.value then
+ if route.value.host then
+ route.value.host = str_lower(route.value.host)
+ elseif route.value.hosts then
+ for i, v in ipairs(route.value.hosts) do
+ route.value.hosts[i] = str_lower(v)
+ end
+ end
+
+ apisix_upstream.filter_upstream(route.value.upstream, route)
+ end
+
+ core.log.info("filter route: ", core.json.delay_encode(route, true))
+
+ --load_full_data()'s filter() goes here. create radixtree while etcd compacts
+
+ if size then
+ if size == #pre_route_obj.values then
Review Comment:
Reloading data does not necessarily mean that the quantity will be the same, the data may change.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] ranxuxin001 commented on a diff in pull request #9334: feat: incremental route radixtree update
Posted by "ranxuxin001 (via GitHub)" <gi...@apache.org>.
ranxuxin001 commented on code in PR #9334:
URL: https://github.com/apache/apisix/pull/9334#discussion_r1205052561
##########
apisix/http/router/radixtree_uri.lua:
##########
@@ -17,33 +17,15 @@
local require = require
local core = require("apisix.core")
local base_router = require("apisix.http.route")
-local get_services = require("apisix.http.service").services
-local cached_router_version
-local cached_service_version
+local router = require("apisix.router")
local _M = {version = 0.2}
- local uri_routes = {}
- local uri_router
local match_opts = {}
function _M.match(api_ctx)
local user_routes = _M.user_routes
- local _, service_version = get_services()
- if not cached_router_version or cached_router_version ~= user_routes.conf_version
- or not cached_service_version or cached_service_version ~= service_version
- then
- uri_router = base_router.create_radixtree_uri_router(user_routes.values,
Review Comment:
apisix/http/router/radixtree_host_uri.lua is done.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9334: feat: incremental route radixtree update
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9334:
URL: https://github.com/apache/apisix/pull/9334#discussion_r1196116254
##########
apisix/http/router/radixtree_uri.lua:
##########
@@ -17,33 +17,15 @@
local require = require
local core = require("apisix.core")
local base_router = require("apisix.http.route")
-local get_services = require("apisix.http.service").services
-local cached_router_version
-local cached_service_version
+local router = require("apisix.router")
local _M = {version = 0.2}
- local uri_routes = {}
- local uri_router
local match_opts = {}
function _M.match(api_ctx)
local user_routes = _M.user_routes
Review Comment:
`user_routes` should be removed too?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] lingliy commented on pull request #9334: feat: incremental route radixtree update
Posted by "lingliy (via GitHub)" <gi...@apache.org>.
lingliy commented on PR #9334:
URL: https://github.com/apache/apisix/pull/9334#issuecomment-1538229036
I also really need this incremental update feature, there are two puzzles
1. According to the routing semantics of apisix, when route or service changes, the route is rebuilt, but service changes are not observed. How is the incremental update implemented
2.radixtree_host_uri.lua, radixtree_uri_with_parameter.lua, these two related incremental updates do not seem to be implemented
Finally, thank you very much for the implementation of feature
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] ranxuxin001 closed pull request #9334: feat: incremental route radixtree update
Posted by "ranxuxin001 (via GitHub)" <gi...@apache.org>.
ranxuxin001 closed pull request #9334: feat: incremental route radixtree update
URL: https://github.com/apache/apisix/pull/9334
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] lingliy commented on pull request #9334: feat: incremental route radixtree update
Posted by "lingliy (via GitHub)" <gi...@apache.org>.
lingliy commented on PR #9334:
URL: https://github.com/apache/apisix/pull/9334#issuecomment-1541347650
> I also really need this incremental update feature, there are two puzzles
>
> 1. According to the routing semantics of apisix, when route or service changes, the route is rebuilt, but service changes are not observed. How is the incremental update implemented
> 2.radixtree_host_uri.lua, radixtree_uri_with_parameter.lua, these two related incremental updates do not seem to be implemented
> Finally, thank you very much for the implementation of feature
I see. I read the semantics of version 2.9, and routes will not be rebuilt after service changes. Later versions will appear
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] ranxuxin001 commented on pull request #9334: feat: incremental route radixtree update
Posted by "ranxuxin001 (via GitHub)" <gi...@apache.org>.
ranxuxin001 commented on PR #9334:
URL: https://github.com/apache/apisix/pull/9334#issuecomment-1541331564
> > I also really need this incremental update feature, there are two puzzles
> >
> > 1. According to the routing semantics of apisix, when route or service changes, the route is rebuilt, but service changes are not observed. How is the incremental update implemented
> > 2.radixtree_host_uri.lua, radixtree_uri_with_parameter.lua, these two related incremental updates do not seem to be implemented
> > Finally, thank you very much for the implementation of feature
>
> Thank you to send request. I wrote this patch based on apisix 2.9. The radixtree_uri.lua runs as default. So I mainly wrote codes for this strategy. Later I will wrote codes based on radixtree_host_uri.lua as the default strategy of apisix 3.3.
>
> I deleted codes rebuilding whote tree when version changed because the modifying routes codes are moved in the filter() called by sync_data().
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9334: feat: incremental route radixtree update
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9334:
URL: https://github.com/apache/apisix/pull/9334#discussion_r1196102608
##########
apisix/router.lua:
##########
@@ -22,34 +22,147 @@ local plugin_checker = require("apisix.plugin").plugin_checker
local str_lower = string.lower
local error = error
local ipairs = ipairs
-
+local sub_str = string.sub
+local table = require("apisix.core.table")
+local json = require("apisix.core.json")
local _M = {version = 0.3}
+local function short_key(self, str)
+ return sub_str(str, #self.key + 2)
+end
-local function filter(route)
+local function filter(route, pre_route_obj, size)
route.orig_modifiedIndex = route.modifiedIndex
- route.update_count = 0
+ route.update_count = 0
route.has_domain = false
- if not route.value then
+ if route.value then
+ if route.value.host then
+ route.value.host = str_lower(route.value.host)
+ elseif route.value.hosts then
+ for i, v in ipairs(route.value.hosts) do
+ route.value.hosts[i] = str_lower(v)
+ end
+ end
+
+ apisix_upstream.filter_upstream(route.value.upstream, route)
+ end
+
+ core.log.info("filter route: ", core.json.delay_encode(route, true))
+
+ --load_full_data()'s filter() goes here. create radixtree while etcd compacts
+
+ if size then
+ if size == #pre_route_obj.values then
Review Comment:
Reloading data does not necessarily mean that the quantity will be the same, the data may change.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9334: feat: incremental route radixtree update
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9334:
URL: https://github.com/apache/apisix/pull/9334#discussion_r1196113194
##########
apisix/router.lua:
##########
@@ -22,34 +22,147 @@ local plugin_checker = require("apisix.plugin").plugin_checker
local str_lower = string.lower
local error = error
local ipairs = ipairs
-
+local sub_str = string.sub
+local table = require("apisix.core.table")
+local json = require("apisix.core.json")
local _M = {version = 0.3}
+local function short_key(self, str)
+ return sub_str(str, #self.key + 2)
+end
-local function filter(route)
+local function filter(route, pre_route_obj, size)
Review Comment:
The function prototype is too obscure, `pre_route_obj` may contain different types. It's better to split the radix tree reconstruction into another function, e.g. `filter_all_routes`.
##########
apisix/router.lua:
##########
@@ -84,6 +198,7 @@ function _M.http_init_worker()
local router_http = require("apisix.http.router." .. router_http_name)
attach_http_router_common_methods(router_http)
router_http.init_worker(filter)
+
Review Comment:
extra blank line?
##########
apisix/router.lua:
##########
@@ -70,6 +183,7 @@ local function attach_http_router_common_methods(http_router)
end
end
+local uri_routes = {}
Review Comment:
dead code?
##########
apisix/http/router/radixtree_uri.lua:
##########
@@ -17,33 +17,15 @@
local require = require
local core = require("apisix.core")
local base_router = require("apisix.http.route")
-local get_services = require("apisix.http.service").services
-local cached_router_version
-local cached_service_version
+local router = require("apisix.router")
local _M = {version = 0.2}
- local uri_routes = {}
- local uri_router
local match_opts = {}
function _M.match(api_ctx)
local user_routes = _M.user_routes
Review Comment:
should be removed too?
##########
apisix/router.lua:
##########
@@ -22,34 +22,147 @@ local plugin_checker = require("apisix.plugin").plugin_checker
local str_lower = string.lower
local error = error
local ipairs = ipairs
-
+local sub_str = string.sub
+local table = require("apisix.core.table")
+local json = require("apisix.core.json")
local _M = {version = 0.3}
+local function short_key(self, str)
+ return sub_str(str, #self.key + 2)
+end
-local function filter(route)
+local function filter(route, pre_route_obj, size)
route.orig_modifiedIndex = route.modifiedIndex
- route.update_count = 0
+ route.update_count = 0
Review Comment:
extra space.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] ranxuxin001 commented on pull request #9334: feat: incremental route radixtree update
Posted by "ranxuxin001 (via GitHub)" <gi...@apache.org>.
ranxuxin001 commented on PR #9334:
URL: https://github.com/apache/apisix/pull/9334#issuecomment-1541330929
> I also really need this incremental update feature, there are two puzzles
>
> 1. According to the routing semantics of apisix, when route or service changes, the route is rebuilt, but service changes are not observed. How is the incremental update implemented
> 2.radixtree_host_uri.lua, radixtree_uri_with_parameter.lua, these two related incremental updates do not seem to be implemented
> Finally, thank you very much for the implementation of feature
Thank you to send request. I wrote this patch based on apisix 2.9. The radixtree_uri.lua runs as default. So I mainly wrote codes for this strategy. Later I will wrote codes based on radixtree_host_uri.lua.
I deleted codes rebuilding whote tree when version changed because the modifying routes codes are moved in the filter() called by sync_data().
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9334: feat: incremental route radixtree update
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9334:
URL: https://github.com/apache/apisix/pull/9334#discussion_r1196150066
##########
apisix/http/router/radixtree_uri.lua:
##########
@@ -17,33 +17,15 @@
local require = require
local core = require("apisix.core")
local base_router = require("apisix.http.route")
-local get_services = require("apisix.http.service").services
-local cached_router_version
-local cached_service_version
+local router = require("apisix.router")
local _M = {version = 0.2}
- local uri_routes = {}
- local uri_router
local match_opts = {}
function _M.match(api_ctx)
local user_routes = _M.user_routes
- local _, service_version = get_services()
- if not cached_router_version or cached_router_version ~= user_routes.conf_version
- or not cached_service_version or cached_service_version ~= service_version
- then
- uri_router = base_router.create_radixtree_uri_router(user_routes.values,
Review Comment:
Also, need to handle below files:
```
apisix/http/router/radixtree_uri_with_parameter.lua
apisix/http/router/radixtree_host_uri.lua
```
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9334: feat: incremental route radixtree update
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9334:
URL: https://github.com/apache/apisix/pull/9334#discussion_r1196113194
##########
apisix/router.lua:
##########
@@ -22,34 +22,147 @@ local plugin_checker = require("apisix.plugin").plugin_checker
local str_lower = string.lower
local error = error
local ipairs = ipairs
-
+local sub_str = string.sub
+local table = require("apisix.core.table")
+local json = require("apisix.core.json")
local _M = {version = 0.3}
+local function short_key(self, str)
+ return sub_str(str, #self.key + 2)
+end
-local function filter(route)
+local function filter(route, pre_route_obj, size)
Review Comment:
The function prototype is too obscure, `pre_route_obj` may contain different types. It's better to split the radix tree reconstruction into another function, e.g. `filter_all_routes`.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org