kingluo commented on code in PR #9692:
URL: https://github.com/apache/apisix/pull/9692#discussion_r1252573136
##########
apisix/http/router/radixtree_uri.lua:
##########
@@ -18,24 +18,128 @@ local require = require
local core = require("apisix.core")
local base_router = require("apisix.http.route")
local get_services = require("apisix.http.service").services
+local apisix_router = require("apisix.router")
+local json = require("apisix.core.json")
+local table = require("apisix.core.table")
local cached_router_version
local cached_service_version
+local uri_routes = {}
+local uri_router
+local match_opts = {}
local _M = {version = 0.2}
- local uri_routes = {}
- local uri_router
- local match_opts = {}
+local function incremental_operate_radixtree(routes)
+ local sync_tb = apisix_router.sync_tb
+ if apisix_router.need_create_radixtree then
+ uri_router = base_router.create_radixtree_uri_router(routes,
uri_routes, false)
+ apisix_router.need_create_radixtree = false
+ for k, _ in pairs(sync_tb) do
+ sync_tb[k] = nil
+ end
+ return
+ end
+
+ local op, route, last_route, err
+ local cur_tmp, last_tmp = {}, {}
+ local router_opts = {
+ no_param_match = true
+ }
+ for k, v in pairs(sync_tb) do
+ op = sync_tb[k]["op"]
+ route = sync_tb[k]["cur_route"]
+ last_route = sync_tb[k]["last_route"]
+ cur_tmp = {}
+ last_tmp = {}
+
+ if route and route.value then
+ local status = table.try_read_attr(route, "value", "status")
+ if status and status == 0 then
+ return
+ end
+
+ local filter_fun, err
+ if route.value.filter_func then
+ filter_fun, err = loadstring(
+ "return " .. route.value.filter_func,
+ "router#" .. route.value.id
+ )
+ if not filter_fun then
+ core.log.error("failed to load filter function: ", err, "
route id", route.value.id)
+ return
+ end
+
+ filter_fun = filter_fun()
+ end
+
+ cur_tmp = {
+ id = route.value.id,
+ paths = route.value.uris or route.value.uri,
+ methods = route.value.methods,
+ priority = route.value.priority,
+ hosts = route.value.hosts or route.value.host,
+ remote_addrs = route.value.remote_addrs or
route.value.remote_addr,
+ vars = route.value.vars,
+ filter_fun = filter_fun,
+ handler = function(api_ctx, match_opts)
+ api_ctx.matched_params = nil
+ api_ctx.matched_route = route
+ api_ctx.curr_req_matched = match_opts.matched
+ end
+ }
+ end
+
+ if last_route and last_route.value then
+ last_tmp = {
+ id = last_route.value.id,
+ paths = last_route.value.uris or last_route.value.uri,
+ methods = last_route.value.methods,
+ priority = last_route.value.priority,
+ hosts = last_route.value.hosts or last_route.value.host,
+ remote_addrs = last_route.value.remote_addrs or
last_route.value.remote_addr,
+ vars = last_route.value.vars
+ }
+ end
+
+ if op == "update" then
+ core.log.notice("update routes watched from etcd into radixtree.",
json.encode(route))
+ err = uri_router:update_route(last_tmp, cur_tmp, router_opts)
+ if err ~= nil then
+ core.log.error("update a route into radixtree failed.",
json.encode(route), err)
+ return
+ end
+ elseif op == "create" then
+ core.log.notice("create routes watched from etcd into radixtree.",
json.encode(route))
+ err = uri_router:add_route(cur_tmp, router_opts)
+ if err ~= nil then
+ core.log.error("add routes into radixtree failed.",
json.encode(route), err)
+ return
+ end
+ elseif op == "delete" then
+ core.log.notice("delete routes watched from etcd into radixtree.",
json.encode(last_route))
+ err = uri_router:delete_route(last_tmp, router_opts)
+ if err ~= nil then
+ core.log.error("delete a route into radixtree failed.",
json.encode(last_route), err)
+ return
+ end
+ end
+
+ sync_tb[k] = nil
+ end
+
+ apisix_router.sync_tb = sync_tb
Review Comment:
no need to reassign the table? because they point to the same table.
##########
apisix/http/router/radixtree_uri.lua:
##########
@@ -18,24 +18,128 @@ local require = require
local core = require("apisix.core")
local base_router = require("apisix.http.route")
local get_services = require("apisix.http.service").services
+local apisix_router = require("apisix.router")
+local json = require("apisix.core.json")
+local table = require("apisix.core.table")
local cached_router_version
local cached_service_version
+local uri_routes = {}
+local uri_router
+local match_opts = {}
local _M = {version = 0.2}
- local uri_routes = {}
- local uri_router
- local match_opts = {}
+local function incremental_operate_radixtree(routes)
+ local sync_tb = apisix_router.sync_tb
+ if apisix_router.need_create_radixtree then
+ uri_router = base_router.create_radixtree_uri_router(routes,
uri_routes, false)
+ apisix_router.need_create_radixtree = false
+ for k, _ in pairs(sync_tb) do
+ sync_tb[k] = nil
+ end
+ return
+ end
+
+ local op, route, last_route, err
+ local cur_tmp, last_tmp = {}, {}
+ local router_opts = {
+ no_param_match = true
+ }
+ for k, v in pairs(sync_tb) do
+ op = sync_tb[k]["op"]
+ route = sync_tb[k]["cur_route"]
+ last_route = sync_tb[k]["last_route"]
+ cur_tmp = {}
+ last_tmp = {}
+
+ if route and route.value then
+ local status = table.try_read_attr(route, "value", "status")
+ if status and status == 0 then
+ return
+ end
+
+ local filter_fun, err
+ if route.value.filter_func then
+ filter_fun, err = loadstring(
+ "return " .. route.value.filter_func,
+ "router#" .. route.value.id
+ )
+ if not filter_fun then
+ core.log.error("failed to load filter function: ", err, "
route id", route.value.id)
+ return
+ end
+
+ filter_fun = filter_fun()
+ end
+
+ cur_tmp = {
+ id = route.value.id,
+ paths = route.value.uris or route.value.uri,
+ methods = route.value.methods,
+ priority = route.value.priority,
+ hosts = route.value.hosts or route.value.host,
+ remote_addrs = route.value.remote_addrs or
route.value.remote_addr,
+ vars = route.value.vars,
+ filter_fun = filter_fun,
+ handler = function(api_ctx, match_opts)
+ api_ctx.matched_params = nil
+ api_ctx.matched_route = route
+ api_ctx.curr_req_matched = match_opts.matched
+ end
+ }
+ end
+
+ if last_route and last_route.value then
+ last_tmp = {
+ id = last_route.value.id,
+ paths = last_route.value.uris or last_route.value.uri,
+ methods = last_route.value.methods,
+ priority = last_route.value.priority,
+ hosts = last_route.value.hosts or last_route.value.host,
+ remote_addrs = last_route.value.remote_addrs or
last_route.value.remote_addr,
+ vars = last_route.value.vars
+ }
+ end
+
+ if op == "update" then
+ core.log.notice("update routes watched from etcd into radixtree.",
json.encode(route))
+ err = uri_router:update_route(last_tmp, cur_tmp, router_opts)
+ if err ~= nil then
+ core.log.error("update a route into radixtree failed.",
json.encode(route), err)
+ return
+ end
+ elseif op == "create" then
+ core.log.notice("create routes watched from etcd into radixtree.",
json.encode(route))
+ err = uri_router:add_route(cur_tmp, router_opts)
+ if err ~= nil then
+ core.log.error("add routes into radixtree failed.",
json.encode(route), err)
+ return
+ end
+ elseif op == "delete" then
+ core.log.notice("delete routes watched from etcd into radixtree.",
json.encode(last_route))
+ err = uri_router:delete_route(last_tmp, router_opts)
+ if err ~= nil then
+ core.log.error("delete a route into radixtree failed.",
json.encode(last_route), err)
+ return
+ end
+ end
+
+ sync_tb[k] = nil
Review Comment:
clear the whole table after the loop?
##########
apisix/http/router/radixtree_host_uri.lua:
##########
@@ -29,81 +32,246 @@ local cached_router_version
local cached_service_version
local host_router
local only_uri_router
+local host_routes = {}
+local only_uri_routes = {}
local _M = {version = 0.1}
-local function push_host_router(route, host_routes, only_uri_routes)
- if type(route) ~= "table" then
+local function empty_func() end
+
+
+local function push_host_router(route, host_routes, only_uri_routes,
all_hosts, op, rdx_rt, pre_route, pre_rdx_rt)
+ if route and type(route) ~= "table" then
return
end
- local filter_fun, err
- if route.value.filter_func then
- filter_fun, err = loadstring(
- "return " .. route.value.filter_func,
- "router#" .. route.value.id)
- if not filter_fun then
- core.log.error("failed to load filter function: ", err,
- " route id: ", route.value.id)
- return
+ local radixtree_route, pre_radixtree_route = {}, {}
+ local hosts
+ if route and route.value then
+ hosts = route.value.hosts
+ if not hosts then
+ if route.value.host then
+ hosts = {route.value.host}
+ elseif route.value.service_id then
+ local service = service_fetch(route.value.service_id)
+ if not service then
+ core.log.error("failed to fetch service configuration by ",
+ "id: ", route.value.service_id)
+ -- we keep the behavior that missing service won't affect
the route matching
+ else
+ hosts = service.value.hosts
+ end
+ end
+ end
+
+ local filter_fun, err
+ if route.value and route.value.filter_func then
+ filter_fun, err = loadstring(
+ "return " .. route.value.filter_func,
+ "router#" .. route.value.id)
+ if not filter_fun then
+ core.log.error("failed to load filter function: ", err,
+ " route id: ", route.value.id)
+ return
+ end
+
+ filter_fun = filter_fun()
+ end
+
+ radixtree_route = {
+ id = route.value.id,
+ paths = route.value.uris or route.value.uri,
+ methods = route.value.methods,
+ priority = route.value.priority,
+ remote_addrs = route.value.remote_addrs
+ or route.value.remote_addr,
+ vars = route.value.vars,
+ filter_fun = filter_fun,
+ handler = function (api_ctx, match_opts)
+ api_ctx.matched_params = nil
+ api_ctx.matched_route = route
+ api_ctx.curr_req_matched = match_opts.matched
+ api_ctx.real_curr_req_matched_path = match_opts.matched._path
+ end
+ }
+
+ if rdx_rt ~= nil then
+ for k, v in pairs(radixtree_route) do
+ rdx_rt[k] = v
+ end
end
+ end
- filter_fun = filter_fun()
+ if hosts == nil and all_hosts == nil then
+ core.table.insert(only_uri_routes, radixtree_route)
+ return
end
- local hosts = route.value.hosts
- if not hosts then
- if route.value.host then
- hosts = {route.value.host}
- elseif route.value.service_id then
- local service = service_fetch(route.value.service_id)
- if not service then
- core.log.error("failed to fetch service configuration by ",
- "id: ", route.value.service_id)
- -- we keep the behavior that missing service won't affect the
route matching
- else
- hosts = service.value.hosts
+ local pre_hosts
+ if pre_route and pre_route.value then
+ pre_hosts = pre_route.value.hosts
+ if not pre_hosts then
+ if pre_route.value.host then
+ pre_hosts = {pre_route.value.host}
+ elseif pre_route.value.service_id then
+ local service = service_fetch(pre_route.value.service_id)
+ if not service then
+ core.log.error("failed to fetch service configuration by ",
+ "id: ", pre_route.value.service_id)
+ -- we keep the behavior that missing service won't affect
the route matching
+ else
+ pre_hosts = service.value.hosts
+ end
+ end
+ end
+
+ local filter_fun, err
+ if pre_route.value and pre_route.value.filter_func then
+ filter_fun, err = loadstring(
+ "return " .. pre_route.value.filter_func,
+ "router#" .. pre_route.value.id)
+ if not filter_fun then
+ core.log.error("failed to load filter function: ", err,
+ " route id: ", pre_route.value.id)
+ return
+ end
+
+ filter_fun = filter_fun()
+ end
+
+ pre_radixtree_route = {
+ id = pre_route.value.id,
+ paths = pre_route.value.uris or pre_route.value.uri,
+ methods = pre_route.value.methods,
+ priority = pre_route.value.priority,
+ remote_addrs = pre_route.value.remote_addrs
+ or pre_route.value.remote_addr,
+ vars = pre_route.value.vars,
+ filter_fun = filter_fun,
+ handler = function (api_ctx, match_opts)
+ api_ctx.matched_params = nil
+ api_ctx.matched_route = pre_route
+ api_ctx.curr_req_matched = match_opts.matched
+ api_ctx.real_curr_req_matched_path = match_opts.matched._path
+ end
+ }
+
+ if pre_rdx_rt ~= nil then
+ for k, v in pairs(pre_radixtree_route) do
+ pre_rdx_rt[k] = v
end
end
end
- local radixtree_route = {
- paths = route.value.uris or route.value.uri,
- methods = route.value.methods,
- priority = route.value.priority,
- remote_addrs = route.value.remote_addrs
- or route.value.remote_addr,
- vars = route.value.vars,
- filter_fun = filter_fun,
- handler = function (api_ctx, match_opts)
- api_ctx.matched_params = nil
- api_ctx.matched_route = route
- api_ctx.curr_req_matched = match_opts.matched
- api_ctx.real_curr_req_matched_path = match_opts.matched._path
+ if all_hosts ~= nil then
+ all_hosts["host"] = hosts
+ all_hosts["pre_host"] = pre_hosts
+ end
+
+ local pre_t = {}
+ if pre_hosts then
+ for i, h in ipairs(pre_hosts) do
+ local rev_h = h:reverse()
+ pre_t[rev_h] = 1
end
- }
+ end
- if hosts == nil then
- core.table.insert(only_uri_routes, radixtree_route)
- return
+ local t = {}
+ if hosts then
+ for i, h in ipairs(hosts) do
+ local rev_h = h:reverse()
+ t[rev_h] = 1
+ end
+ end
+
+ local comm = {}
+ for k, v in pairs(pre_t) do
+ if t[k] ~= nil then
+ tab_insert(comm, k)
+ pre_t[k] = nil
+ t[k] = nil
+ end
+ end
+
+ for _, j in ipairs(comm) do
+ local routes = host_routes[j]
+ if routes == nil then
+ core.log.error("no routes array for reverse host in the map.", j)
+ return
+ end
+
+ local found = false
+ for i, r in ipairs(routes) do
+ if r.id == radixtree_route.id then
+ routes[i] = radixtree_route
+ found = true
+ if op then
+ table.insert(op["upd"], j)
+ end
+ break
+ end
+ end
+
+ if not found then
+ core.log.error("cannot find the route in common host's table.", j,
radixtree_route.id)
+ return
+ end
end
- for i, host in ipairs(hosts) do
- local host_rev = host:reverse()
- if not host_routes[host_rev] then
- host_routes[host_rev] = {radixtree_route}
+ for k, v in pairs(pre_t) do
+ local routes = host_routes[k]
+ if routes == nil then
+ core.log.error("no routes array for reverse host in the map.", k)
+ return
+ end
+
+ local found = false
+ for i, r in ipairs(routes) do
+ if r.id == pre_radixtree_route.id then
+ table.remove(routes, i)
+ found = true
+ break
+ end
+ end
+
+ if not found then
+ core.log.error("cannot find the route in previous host's table.",
k, pre_radixtree_route.id)
+ return
+ end
+
+ if #routes == 0 then
+ host_routes[k] = nil
+ if op then
+ core.log.error("###################del####", k)
Review Comment:
remove the debug logging.
##########
apisix/http/router/radixtree_uri_with_parameter.lua:
##########
@@ -18,24 +18,128 @@ local require = require
local core = require("apisix.core")
local base_router = require("apisix.http.route")
local get_services = require("apisix.http.service").services
+local apisix_router = require("apisix.router")
+local json = require("apisix.core.json")
+local table = require("apisix.core.table")
local cached_router_version
local cached_service_version
+local uri_routes = {}
+local uri_router
+local match_opts = {}
local _M = {}
- local uri_routes = {}
- local uri_router
- local match_opts = {}
+local function incremental_operate_radixtree(routes)
Review Comment:
We should put this function in radixtree_uri.lua or elsewhere and reuse it
here, instead of redefine? They are the same except for the no_param_match.
##########
apisix/router.lua:
##########
@@ -22,30 +22,95 @@ local plugin_checker =
require("apisix.plugin").plugin_checker
local str_lower = string.lower
local error = error
local ipairs = ipairs
+local table = require("apisix.core.table")
+local json = require("apisix.core.json")
+local sub_str = string.sub
local _M = {version = 0.3}
+_M.need_create_radixtree = true
-local function filter(route)
+
+local function short_key(self, str)
+ return sub_str(str, #self.key + 2)
+end
+
+
+local function filter(route, pre_route_or_size, obj)
route.orig_modifiedIndex = route.modifiedIndex
route.update_count = 0
route.has_domain = false
- if not route.value then
- return
+ 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
- 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)
+ core.log.info("filter route: ", core.json.delay_encode(route, true))
+
+ if not obj then
+ return
+ end
+ --save sync route and operation type into a map
+ if type(pre_route_or_size) == "number" then
+ if pre_route_or_size == #obj.values then
+ _M.need_create_radixtree = true
end
+ return
end
- apisix_upstream.filter_upstream(route.value.upstream, route)
+ local key
+ if obj.single_item then
+ key = obj.key
+ else
+ key = short_key(obj, route.key)
+ end
+
+ local sync_tb = _M.sync_tb
+ if pre_route_or_size then
+ if route.value then
+ --update route
+ core.log.notice("update routes watched from etcd into radixtree.",
json.encode(route))
+ if not sync_tb[route.value.id] then
+ sync_tb[route.value.id] = {op = "update", last_route =
pre_route_or_size, cur_route = route}
+ elseif sync_tb[route.value.id]["op"] == "update" then
+ sync_tb[route.value.id] = {op = "update", last_route =
sync_tb[route.value.id]["last_route"], cur_route = route}
+ elseif sync_tb[route.value.id]["op"] == "create" then
+ sync_tb[route.value.id] = {op = "create", cur_route = route}
+ end
+ else
+ --delete route
+ core.log.notice("delete routes watched from etcd into radixtree.",
json.encode(route))
+ if not sync_tb[key] then
+ sync_tb[key] = {op = "delete", last_route = pre_route_or_size}
+ elseif sync_tb[key]["op"] == "create" then
+ sync_tb[key] = nil
+ elseif sync_tb[key]["op"] == "update" then
+ sync_tb[key] = {op = "delete", last_route =
sync_tb[route.value.id]["last_route"]}
+ end
+ end
+ elseif route.value then
+ --create route
+ core.log.notice("create routes watched from etcd into radixtree.",
json.encode(route))
+ if not sync_tb[route.value.id] then
+ sync_tb[route.value.id] = {op = "create", cur_route = route}
+ elseif sync_tb[route.value.id]["op"] == "delete" then
+ sync_tb[route.value.id] = {op = "update", cur_route = route,
last_route = sync_tb[route.value.id]["last_route"]}
+ end
+ else
+ core.log.error("invalid operation type for a route.", route.key)
+ return
+ end
+ _M.sync_tb = sync_tb
Review Comment:
remove this statement, and ditto.
##########
apisix/http/router/radixtree_uri.lua:
##########
@@ -18,24 +18,128 @@ local require = require
local core = require("apisix.core")
local base_router = require("apisix.http.route")
local get_services = require("apisix.http.service").services
+local apisix_router = require("apisix.router")
+local json = require("apisix.core.json")
+local table = require("apisix.core.table")
local cached_router_version
local cached_service_version
+local uri_routes = {}
+local uri_router
+local match_opts = {}
local _M = {version = 0.2}
- local uri_routes = {}
- local uri_router
- local match_opts = {}
+local function incremental_operate_radixtree(routes)
+ local sync_tb = apisix_router.sync_tb
+ if apisix_router.need_create_radixtree then
+ uri_router = base_router.create_radixtree_uri_router(routes,
uri_routes, false)
+ apisix_router.need_create_radixtree = false
+ for k, _ in pairs(sync_tb) do
+ sync_tb[k] = nil
+ end
+ return
+ end
+
+ local op, route, last_route, err
+ local cur_tmp, last_tmp = {}, {}
+ local router_opts = {
+ no_param_match = true
+ }
+ for k, v in pairs(sync_tb) do
+ op = sync_tb[k]["op"]
+ route = sync_tb[k]["cur_route"]
+ last_route = sync_tb[k]["last_route"]
+ cur_tmp = {}
+ last_tmp = {}
+
+ if route and route.value then
+ local status = table.try_read_attr(route, "value", "status")
+ if status and status == 0 then
+ return
Review Comment:
continue the loop? You could use goto statement.
--
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]
