From cbe1c431bf0da0eb1758959e7c23a64c1bd46a6e Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Mon, 24 Oct 2022 15:38:18 +0800 Subject: [PATCH 1/6] feat: renew route lrucache when the routes change --- apisix/plugins/ai.lua | 38 +++++++++++++++--------------- t/plugin/ai2.t | 54 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 18 deletions(-) diff --git a/apisix/plugins/ai.lua b/apisix/plugins/ai.lua index 699b140b477f..43bb397e36ad 100644 --- a/apisix/plugins/ai.lua +++ b/apisix/plugins/ai.lua @@ -24,6 +24,7 @@ local ngx = ngx local is_http = ngx.config.subsystem == "http" local enable_keepalive = balancer.enable_keepalive and is_http local is_apisix_or, response = pcall(require, "resty.apisix.response") +local base_router = require("apisix.http.route") local ipairs = ipairs local pcall = pcall local loadstring = loadstring @@ -46,11 +47,7 @@ return function(ctx) end ]] -local route_lrucache = core.lrucache.new({ - -- TODO: we need to set the cache size by count of routes - -- if we have done this feature, we need to release the origin lrucache - count = 512 -}) +local route_lrucache local schema = {} @@ -70,21 +67,20 @@ local orig_http_balancer_phase = apisix.http_balancer_phase local default_keepalive_pool = {} -local function match_route(ctx) - orig_router_match(ctx) - return ctx.matched_route or false +local function match_route(uri_router, match_opts, api_ctx) + orig_router_match(uri_router, match_opts, api_ctx) + return api_ctx.matched_route or false end -local function ai_match(ctx) - local key = get_cache_key_func(ctx) +local function ai_match_uri(uri_router, match_opts, api_ctx) + local key = get_cache_key_func(api_ctx) core.log.info("route cache key: ", key) - local ver = router.router_http.user_routes.conf_version - local route_cache = route_lrucache(key, ver, - match_route, ctx) + local route_cache = route_lrucache(key, nil, + match_route, uri_router, match_opts, api_ctx) -- if the version has not changed, use the cached route if route_cache then - ctx.matched_route = route_cache + api_ctx.matched_route = route_cache end end @@ -224,15 +220,20 @@ local function routes_analyze(routes) or route_flags["service_id"] or route_flags["plugin_config_id"] or global_rules_flag then - router.router_http.match = orig_router_match + base_router.match_uri = orig_router_match else core.log.info("use ai plane to match route") - router.router_http.match = ai_match + base_router.match_uri = ai_match_uri + local count = #routes + 3000 + core.log.info("renew route cache: count=", count) + route_lrucache = core.lrucache.new({ + count = count + }) local ok, err = gen_get_cache_key_func(route_flags) if not ok then core.log.error("generate get_cache_key_func failed:", err) - router.router_http.match = orig_router_match + base_router.match_uri = orig_router_match end end @@ -280,7 +281,8 @@ end function _M.init_worker() - orig_router_match = router.router_http.match + orig_router_match = base_router.match_uri end + return _M diff --git a/t/plugin/ai2.t b/t/plugin/ai2.t index 2a54ae97f08a..7897af66646b 100644 --- a/t/plugin/ai2.t +++ b/t/plugin/ai2.t @@ -375,3 +375,57 @@ do body filter run before_proxy phase balancer_ip : 127.0.0.1 --- no_error_log enable sample upstream + + + +=== TEST 6: renew route cache +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local http = require "resty.http" + local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" + for k = 1, 2 do + local code, body = t('/apisix/admin/routes/' .. k, + ngx.HTTP_PUT, + [[{ + "host": "127.0.0.1", + "methods": ["GET"], + "plugins": { + "proxy-rewrite": { + "uri": "/hello" + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello]] .. k .. [[" + }]] + ) + if code >= 300 then + ngx.status = code + ngx.say(body) + return + end + ngx.sleep(1) + for i = 1, 2 do + local httpc = http.new() + local res, err = httpc:request_uri(uri .. k) + assert(res.status == 200) + if not res then + ngx.log(ngx.ERR, err) + return + end + end + end + ngx.say("done") + } + } +--- response_body +done +--- error_log +renew route cache: count=3001 +renew route cache: count=3002 From 2587cea571d153ade4dd821b1b675afeadc8c850 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Mon, 24 Oct 2022 16:38:10 +0800 Subject: [PATCH 2/6] extract route matching --- apisix/http/router/radixtree_host_uri.lua | 4 ++++ apisix/http/router/radixtree_uri.lua | 5 +++++ .../router/radixtree_uri_with_parameter.lua | 5 +++++ apisix/plugins/ai.lua | 20 +++++++++---------- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/apisix/http/router/radixtree_host_uri.lua b/apisix/http/router/radixtree_host_uri.lua index 51a4cdd600a1..449a6ccdbba7 100644 --- a/apisix/http/router/radixtree_host_uri.lua +++ b/apisix/http/router/radixtree_host_uri.lua @@ -150,7 +150,11 @@ function _M.match(api_ctx) cached_service_version = service_version end + return _M.matching(api_ctx) +end + +function _M.matching(api_ctx) core.table.clear(match_opts) match_opts.method = api_ctx.var.request_method match_opts.remote_addr = api_ctx.var.remote_addr diff --git a/apisix/http/router/radixtree_uri.lua b/apisix/http/router/radixtree_uri.lua index b438ed8d877d..25789a4a3c63 100644 --- a/apisix/http/router/radixtree_uri.lua +++ b/apisix/http/router/radixtree_uri.lua @@ -45,6 +45,11 @@ function _M.match(api_ctx) return true end + return _M.matching(api_ctx) +end + + +function _M.matching(api_ctx) return base_router.match_uri(uri_router, match_opts, api_ctx) end diff --git a/apisix/http/router/radixtree_uri_with_parameter.lua b/apisix/http/router/radixtree_uri_with_parameter.lua index 257017199f27..696c74f0be4b 100644 --- a/apisix/http/router/radixtree_uri_with_parameter.lua +++ b/apisix/http/router/radixtree_uri_with_parameter.lua @@ -45,6 +45,11 @@ function _M.match(api_ctx) return true end + return _M.matching(api_ctx) +end + + +function _M.matching(api_ctx) return base_router.match_uri(uri_router, match_opts, api_ctx) end diff --git a/apisix/plugins/ai.lua b/apisix/plugins/ai.lua index 43bb397e36ad..9a92db27ea84 100644 --- a/apisix/plugins/ai.lua +++ b/apisix/plugins/ai.lua @@ -24,7 +24,6 @@ local ngx = ngx local is_http = ngx.config.subsystem == "http" local enable_keepalive = balancer.enable_keepalive and is_http local is_apisix_or, response = pcall(require, "resty.apisix.response") -local base_router = require("apisix.http.route") local ipairs = ipairs local pcall = pcall local loadstring = loadstring @@ -61,23 +60,23 @@ local _M = { scope = "global", } -local orig_router_match +local orig_router_http_matching local orig_handle_upstream = apisix.handle_upstream local orig_http_balancer_phase = apisix.http_balancer_phase local default_keepalive_pool = {} -local function match_route(uri_router, match_opts, api_ctx) - orig_router_match(uri_router, match_opts, api_ctx) +local function create_router_matching_cache(api_ctx) + orig_router_http_matching(api_ctx) return api_ctx.matched_route or false end -local function ai_match_uri(uri_router, match_opts, api_ctx) +local function ai_router_http_matching(api_ctx) local key = get_cache_key_func(api_ctx) core.log.info("route cache key: ", key) local route_cache = route_lrucache(key, nil, - match_route, uri_router, match_opts, api_ctx) + create_router_matching_cache, api_ctx) -- if the version has not changed, use the cached route if route_cache then api_ctx.matched_route = route_cache @@ -220,10 +219,11 @@ local function routes_analyze(routes) or route_flags["service_id"] or route_flags["plugin_config_id"] or global_rules_flag then - base_router.match_uri = orig_router_match + router.router_http.matching = orig_router_http_matching else core.log.info("use ai plane to match route") - base_router.match_uri = ai_match_uri + router.router_http.matching = ai_router_http_matching + local count = #routes + 3000 core.log.info("renew route cache: count=", count) route_lrucache = core.lrucache.new({ @@ -233,7 +233,7 @@ local function routes_analyze(routes) local ok, err = gen_get_cache_key_func(route_flags) if not ok then core.log.error("generate get_cache_key_func failed:", err) - base_router.match_uri = orig_router_match + router.router_http.matching = orig_router_http_matching end end @@ -281,7 +281,7 @@ end function _M.init_worker() - orig_router_match = base_router.match_uri + orig_router_http_matching = router.router_http.matching end From cf968629de85794f349db6f6ad2f121ed572ad38 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Mon, 24 Oct 2022 16:48:55 +0800 Subject: [PATCH 3/6] adjusting test cases --- t/plugin/ai.t | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++ t/plugin/ai2.t | 54 -------------------------------------------------- 2 files changed, 54 insertions(+), 54 deletions(-) diff --git a/t/plugin/ai.t b/t/plugin/ai.t index f695788ec190..6d04c192041a 100644 --- a/t/plugin/ai.t +++ b/t/plugin/ai.t @@ -875,3 +875,57 @@ done qr/enable sample upstream/ --- grep_error_log_out enable sample upstream + + + +=== TEST 14: renew route cache +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local http = require "resty.http" + local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" + for k = 1, 2 do + local code, body = t('/apisix/admin/routes/' .. k, + ngx.HTTP_PUT, + [[{ + "host": "127.0.0.1", + "methods": ["GET"], + "plugins": { + "proxy-rewrite": { + "uri": "/hello" + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello]] .. k .. [[" + }]] + ) + if code >= 300 then + ngx.status = code + ngx.say(body) + return + end + ngx.sleep(1) + for i = 1, 2 do + local httpc = http.new() + local res, err = httpc:request_uri(uri .. k) + assert(res.status == 200) + if not res then + ngx.log(ngx.ERR, err) + return + end + end + end + ngx.say("done") + } + } +--- response_body +done +--- error_log +renew route cache: count=3001 +renew route cache: count=3002 diff --git a/t/plugin/ai2.t b/t/plugin/ai2.t index 7897af66646b..2a54ae97f08a 100644 --- a/t/plugin/ai2.t +++ b/t/plugin/ai2.t @@ -375,57 +375,3 @@ do body filter run before_proxy phase balancer_ip : 127.0.0.1 --- no_error_log enable sample upstream - - - -=== TEST 6: renew route cache ---- config - location /t { - content_by_lua_block { - local t = require("lib.test_admin").test - local http = require "resty.http" - local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" - for k = 1, 2 do - local code, body = t('/apisix/admin/routes/' .. k, - ngx.HTTP_PUT, - [[{ - "host": "127.0.0.1", - "methods": ["GET"], - "plugins": { - "proxy-rewrite": { - "uri": "/hello" - } - }, - "upstream": { - "nodes": { - "127.0.0.1:1980": 1 - }, - "type": "roundrobin" - }, - "uri": "/hello]] .. k .. [[" - }]] - ) - if code >= 300 then - ngx.status = code - ngx.say(body) - return - end - ngx.sleep(1) - for i = 1, 2 do - local httpc = http.new() - local res, err = httpc:request_uri(uri .. k) - assert(res.status == 200) - if not res then - ngx.log(ngx.ERR, err) - return - end - end - end - ngx.say("done") - } - } ---- response_body -done ---- error_log -renew route cache: count=3001 -renew route cache: count=3002 From ddfcd70d2e5b67e6eedab046e5901f916b1f1fe3 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Mon, 24 Oct 2022 19:35:23 +0800 Subject: [PATCH 4/6] fix unload plugin --- apisix/init.lua | 2 -- apisix/plugins/ai.lua | 23 +++++++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/apisix/init.lua b/apisix/init.lua index 883dbd9abab0..01f4398cc559 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -152,8 +152,6 @@ function _M.http_init_worker() apisix_upstream.init_worker() require("apisix.plugins.ext-plugin.init").init_worker() - -- TODO: need to revisit code layering and avoid similar hacking - require("apisix.plugins.ai").init_worker() local_conf = core.config.local_conf() diff --git a/apisix/plugins/ai.lua b/apisix/plugins/ai.lua index 9a92db27ea84..b8e76f356642 100644 --- a/apisix/plugins/ai.lua +++ b/apisix/plugins/ai.lua @@ -144,6 +144,10 @@ end local function routes_analyze(routes) + if orig_router_http_matching == nil then + orig_router_http_matching = router.router_http.matching + end + local route_flags = core.table.new(0, 16) local route_up_flags = core.table.new(0, 12) for _, route in ipairs(routes) do @@ -280,8 +284,23 @@ function _M.init() end -function _M.init_worker() - orig_router_http_matching = router.router_http.matching +function _M.destroy() + -- TODO: add test cases to cover this function + -- if the ai plugin is disabled at runtime, all functions replaced by the ai plugin are restored + if orig_router_http_matching then + router.router_http.matching = orig_router_http_matching + orig_router_http_matching = nil + end + + if orig_handle_upstream then + apisix.handle_upstream = orig_handle_upstream + orig_handle_upstream = nil + end + + if orig_http_balancer_phase then + apisix.http_balancer_phase = orig_http_balancer_phase + orig_http_balancer_phase = nil + end end From c2a652a8c78bac96a6ca1b55fe8773d935055d77 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Mon, 24 Oct 2022 22:30:38 +0800 Subject: [PATCH 5/6] fix init var phase --- apisix/plugins/ai.lua | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/apisix/plugins/ai.lua b/apisix/plugins/ai.lua index b8e76f356642..0855d5c290a7 100644 --- a/apisix/plugins/ai.lua +++ b/apisix/plugins/ai.lua @@ -61,25 +61,28 @@ local _M = { } local orig_router_http_matching -local orig_handle_upstream = apisix.handle_upstream -local orig_http_balancer_phase = apisix.http_balancer_phase +local orig_handle_upstream +local orig_http_balancer_phase local default_keepalive_pool = {} local function create_router_matching_cache(api_ctx) orig_router_http_matching(api_ctx) - return api_ctx.matched_route or false + return core.table.deepcopy(api_ctx) end local function ai_router_http_matching(api_ctx) local key = get_cache_key_func(api_ctx) core.log.info("route cache key: ", key) - local route_cache = route_lrucache(key, nil, - create_router_matching_cache, api_ctx) + local api_ctx_cache = route_lrucache(key, nil, + create_router_matching_cache, api_ctx) -- if the version has not changed, use the cached route - if route_cache then - api_ctx.matched_route = route_cache + if api_ctx then + api_ctx.matched_route = api_ctx_cache.matched_route + if api_ctx_cache.curr_req_matched then + api_ctx.curr_req_matched = core.table.deepcopy(api_ctx_cache.curr_req_matched) + end end end @@ -148,6 +151,14 @@ local function routes_analyze(routes) orig_router_http_matching = router.router_http.matching end + if orig_handle_upstream == nil then + orig_handle_upstream = apisix.handle_upstream + end + + if orig_http_balancer_phase == nil then + orig_http_balancer_phase = apisix.http_balancer_phase + end + local route_flags = core.table.new(0, 16) local route_up_flags = core.table.new(0, 12) for _, route in ipairs(routes) do From 090d99d3326443fd7258587a98abc9c39aacd34a Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Mon, 24 Oct 2022 22:46:26 +0800 Subject: [PATCH 6/6] use table clone --- apisix/plugins/ai.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apisix/plugins/ai.lua b/apisix/plugins/ai.lua index 0855d5c290a7..46c90ef2e668 100644 --- a/apisix/plugins/ai.lua +++ b/apisix/plugins/ai.lua @@ -81,7 +81,7 @@ local function ai_router_http_matching(api_ctx) if api_ctx then api_ctx.matched_route = api_ctx_cache.matched_route if api_ctx_cache.curr_req_matched then - api_ctx.curr_req_matched = core.table.deepcopy(api_ctx_cache.curr_req_matched) + api_ctx.curr_req_matched = core.table.clone(api_ctx_cache.curr_req_matched) end end end