feat(ai): add dynamic route cache key#8113
Conversation
| router.match = ai_match | ||
|
|
||
| if get_cache_key_func_def_render == nil then | ||
| local template = require("resty.template") |
There was a problem hiding this comment.
can we do this template at top level?
There was a problem hiding this comment.
resty.template depends on nginx.location, but ai.lua would be called in init_by_lua, where no ngx.location, so for the top level case, the require would fail.
| 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}) |
There was a problem hiding this comment.
can we move this logic to one function?
then we can check the return value, and do different thing
membphis
changed the title
membphis
left a comment
membphis
left a comment
There was a problem hiding this comment.
pls add some test cases to check it works fine
| local str = get_cache_key_func_def_render({route_flags = route_flags}) | ||
| local func, err = loadstring(str) | ||
| if func == nil then | ||
| return err |
There was a problem hiding this comment.
oh my god, this is not a good design.
we should use return false, err, the error message can be ignore in some case
| core.log.info("use ai plane to match route") | ||
| router.match = ai_match | ||
|
|
||
| local err = gen_get_cache_key_func(route_flags) |
| return false, err | ||
| else | ||
| local ok | ||
| ok, get_cache_key_func = pcall(func) |
There was a problem hiding this comment.
Better to use a temp var instead of hijacking a module var to hold the potential err message
|
|
||
| local get_cache_key_func_def = [[ | ||
| return function(ctx) | ||
| return ctx.var.uri |
There was a problem hiding this comment.
We can make it a little faster, by using local var = ctx.var to reduce table lookup.
| ngx.sleep(1) | ||
|
|
||
| -- enable | ||
| do |
There was a problem hiding this comment.
Let's exact this common part into a function, instead of repeating it three times.
spacewander
changed the title
| -- 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)) |
There was a problem hiding this comment.
@kingluo after my benchmark, we have to remove this line.
so we have to find a new way of testing.
| local var = ctx.var | ||
| return var.uri | ||
| {% if route_flags["methods"] then %} | ||
| .. "\0" .. var.method |
There was a problem hiding this comment.
the uri, host and method, they should generally not contain #
3 participants
Description
add dynamic route cache key
Checklist