-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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.
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
feat: increment route update for radixtree host uri, radixtree uri and radi… #9692
base: master
Are you sure you want to change the base?
Conversation
…xtree uri with parameter
apisix/http/router/radixtree_uri.lua
Outdated
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use require("table.clear") instead?
apisix/http/router/radixtree_uri.lua
Outdated
if route and route.value then | ||
local status = table.try_read_attr(route, "value", "status") | ||
if status and status == 0 then | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
continue the loop? You could use goto statement.
apisix/http/router/radixtree_uri.lua
Outdated
end | ||
end | ||
|
||
sync_tb[k] = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clear the whole table after the loop?
apisix/http/router/radixtree_uri.lua
Outdated
sync_tb[k] = nil | ||
end | ||
|
||
apisix_router.sync_tb = sync_tb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to reassign the table? because they point to the same table.
|
||
_M.sync_tb = sync_tb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this statement, and ditto.
local uri_routes = {} | ||
local uri_router | ||
local match_opts = {} | ||
local function incremental_operate_radixtree(routes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
if #routes == 0 then | ||
host_routes[k] = nil | ||
if op then | ||
core.log.error("###################del####", k) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the debug logging.
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions. |
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions. |
ping @ranxuxin001 |
ping @ranxuxin001 |
@ranxuxin001 are you available to work on this? We understand that you will be having other things to work for. In case you're not available to work on this, please mention it so that someone else can take this issue up! Thank you again for contributing to apisix! |
hi @nitishfy , my colleague can no longer work on this issue. plz find someone else to handle it. |
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions. |
This PR extends apisix to modify a route without create the whole radixtree everytime.
refer to #9334