Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(ai): add dynamic route cache key #8113

Merged
merged 11 commits into from
Oct 19, 2022
Merged

feat(ai): add dynamic route cache key #8113

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0452755
fix: add dynamic route cache key
kingluo Oct 18, 2022
3ef764e
fix lint
kingluo Oct 18, 2022
37899ff
adjust
kingluo Oct 18, 2022
db0c884
bugfix
kingluo Oct 18, 2022
203242b
bugfix
kingluo Oct 18, 2022
ea16397
bugfix
kingluo Oct 18, 2022
67a15d7
fix PR
kingluo Oct 18, 2022
df380b8
fix PR
kingluo Oct 18, 2022
3e87bf0
fix style
kingluo Oct 19, 2022
7ba7468
fix PR
kingluo Oct 19, 2022
e5b3958
fix PR
kingluo Oct 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 55 additions & 2 deletions apisix/core/ai.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,24 @@
local require = require
local core = require("apisix.core")
local ipairs = ipairs
local pcall = pcall
local loadstring = loadstring
local encode_base64 = ngx.encode_base64

local get_cache_key_func
local get_cache_key_func_def_render

local get_cache_key_func_def = [[
return function(ctx)
return ctx.var.uri
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make it a little faster, by using local var = ctx.var to reduce table lookup.

{% if route_flags["methods"] then %}
.. "\0" .. ctx.var.method
{% end %}
{% if route_flags["host"] then %}
.. "\0" .. ctx.var.host
{% end %}
end
]]

local route_lrucache = core.lrucache.new({
-- TODO: we need to set the cache size by count of routes
Expand All @@ -36,8 +54,8 @@ end


local function ai_match(ctx)
-- TODO: we need to generate cache key dynamically
local key = ctx.var.uri .. "-" .. ctx.var.method .. "-" .. ctx.var.host
local key = get_cache_key_func(ctx)
core.log.info("route cache key: ", core.log.delay_exec(encode_base64, key))
Copy link
Member

@membphis membphis Oct 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kingluo after my benchmark, we have to remove this line.

so we have to find a new way of testing.

local ver = router.user_routes.conf_version
local route_cache = route_lrucache(key, ver,
match_route, ctx)
Expand All @@ -48,10 +66,39 @@ local function ai_match(ctx)
end


local function gen_get_cache_key_func(route_flags)
if get_cache_key_func_def_render == nil then
local template = require("resty.template")
get_cache_key_func_def_render = template.compile(get_cache_key_func_def)
end

local str = get_cache_key_func_def_render({route_flags = route_flags})
local func, err = loadstring(str)
if func == nil then
return err
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh my god, this is not a good design.

we should use return false, err, the error message can be ignore in some case

else
local ok
ok, get_cache_key_func = pcall(func)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to use a temp var instead of hijacking a module var to hold the potential err message

if not ok then
local err = get_cache_key_func
return err
end
end
end


function _M.routes_analyze(routes)
-- TODO: we need to add a option in config.yaml to enable this feature(default is true)
local route_flags = core.table.new(0, 2)
for _, route in ipairs(routes) do
if route.methods then
route_flags["methods"] = true
end

if route.host or route.hosts then
route_flags["host"] = true
end

if route.vars then
route_flags["vars"] = true
end
Expand All @@ -71,6 +118,12 @@ function _M.routes_analyze(routes)
else
core.log.info("use ai plane to match route")
router.match = ai_match

local err = gen_get_cache_key_func(route_flags)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

if err then
core.log.error("generate get_cache_key_func failed:", err)
router.match = orig_router_match
end
end
end

Expand Down
174 changes: 174 additions & 0 deletions t/core/ai.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,3 +474,177 @@ qr/use ai plane to match route/
--- grep_error_log_out
use ai plane to match route
use ai plane to match route



=== TEST 6: route key: uri
--- 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"

local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)

if code >= 300 then
ngx.status = code
ngx.say(body)
return
end
ngx.sleep(1)

-- enable
do
local httpc = http.new()
local res, err = httpc:request_uri(uri)
assert(res.status == 200)
if not res then
ngx.log(ngx.ERR, err)
return
end
end

local httpc = http.new()
local res, err = httpc:request_uri(uri)
assert(res.status == 200)
if not res then
ngx.log(ngx.ERR, err)
return
end

ngx.say("done")
}
}
--- response_body
done
--- error_log
route cache key: L2hlbGxv



=== TEST 7: route key: uri + method
--- 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"

local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"methods": ["GET"],
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)

if code >= 300 then
ngx.status = code
ngx.say(body)
return
end
ngx.sleep(1)

-- enable
do
local httpc = http.new()
local res, err = httpc:request_uri(uri)
assert(res.status == 200)
if not res then
ngx.log(ngx.ERR, err)
return
end
end

local httpc = http.new()
local res, err = httpc:request_uri(uri)
assert(res.status == 200)
if not res then
ngx.log(ngx.ERR, err)
return
end

ngx.say("done")
}
}
--- response_body
done
--- error_log
route cache key: L2hlbGxvAEdFVA==



=== TEST 8: route key: uri + method + host
--- 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"

local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"host": "127.0.0.1",
"methods": ["GET"],
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)

if code >= 300 then
ngx.status = code
ngx.say(body)
return
end
ngx.sleep(1)

-- enable
do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's exact this common part into a function, instead of repeating it three times.

local httpc = http.new()
local res, err = httpc:request_uri(uri)
assert(res.status == 200)
if not res then
ngx.log(ngx.ERR, err)
return
end
end

local httpc = http.new()
local res, err = httpc:request_uri(uri)
assert(res.status == 200)
if not res then
ngx.log(ngx.ERR, err)
return
end

ngx.say("done")
}
}
--- response_body
done
--- error_log
route cache key: L2hlbGxvAEdFVAAxMjcuMC4wLjE=