Skip to content

feat: increment route update for radixtree host uri, radixtree uri and radi…#9692

Closed
ranxuxin001 wants to merge 23 commits into
apache:masterfrom
ranxuxin001:master
Closed

feat: increment route update for radixtree host uri, radixtree uri and radi…#9692
ranxuxin001 wants to merge 23 commits into
apache:masterfrom
ranxuxin001:master

Conversation

@ranxuxin001

Copy link
Copy Markdown

This PR extends apisix to modify a route without create the whole radixtree everytime.

refer to #9334

@ranxuxin001 ranxuxin001 changed the title increment route update for radixtree host uri, radixtree uri and radi… feat: increment route update for radixtree host uri, radixtree uri and radi… Jun 19, 2023
@leslie-tsang leslie-tsang requested a review from kingluo June 19, 2023 06:51
Comment thread apisix/http/router/radixtree_host_uri.lua Outdated
Comment thread apisix/http/router/radixtree_host_uri.lua Outdated
Comment thread apisix/http/router/radixtree_host_uri.lua Outdated
Comment thread 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

Copy link
Copy Markdown
Contributor

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?

Comment thread 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

Copy link
Copy Markdown
Contributor

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.

Comment thread apisix/http/router/radixtree_uri.lua Outdated
end
end

sync_tb[k] = nil

Copy link
Copy Markdown
Contributor

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?

Comment thread apisix/http/router/radixtree_uri.lua Outdated
sync_tb[k] = nil
end

apisix_router.sync_tb = sync_tb

Copy link
Copy Markdown
Contributor

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.

Comment thread apisix/router.lua
return
end

_M.sync_tb = sync_tb

Copy link
Copy Markdown
Contributor

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)

Copy link
Copy Markdown
Contributor

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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove the debug logging.

@github-actions

Copy link
Copy Markdown

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.

@github-actions

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added the stale label Jan 22, 2024
@shreemaan-abhishek

Copy link
Copy Markdown
Contributor

ping @ranxuxin001

@github-actions github-actions Bot removed the stale label Feb 6, 2024
@Revolyssup

Copy link
Copy Markdown
Contributor

ping @ranxuxin001

@nitishfy

Copy link
Copy Markdown

@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!

@TakiJoe

TakiJoe commented May 8, 2024

Copy link
Copy Markdown

@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.

@github-actions

github-actions Bot commented Jul 7, 2024

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added the stale label Jul 7, 2024
@github-actions

github-actions Bot commented Aug 4, 2024

Copy link
Copy Markdown

This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions Bot closed this Aug 4, 2024
@wklken

wklken commented Oct 15, 2024

Copy link
Copy Markdown

Perhaps this pull request should be reconsidered for reopening. I believe it could be highly beneficial for environments where the route configuration frequently changes.

@sergey-jr

Copy link
Copy Markdown

@Revolyssup Could you please reopen this issue? The problem still persists and seems serious. While there is a potential workaround with this patch: #12466, an official fix from the project would be much appreciated.

@Revolyssup

Copy link
Copy Markdown
Contributor

@Revolyssup Could you please reopen this issue? The problem still persists and seems serious. While there is a potential workaround with this patch: #12466, an official fix from the project would be much appreciated.

@Baoyuantop Please take a look

@Baoyuantop

Copy link
Copy Markdown
Contributor

Hi @sergey-jr, thanks for your feedback, I reopened it.

@Baoyuantop Baoyuantop reopened this Oct 23, 2025
@Baoyuantop

Copy link
Copy Markdown
Contributor

Since the original author is no longer able to work on this PR, anyone is welcome to resubmit.

@github-actions github-actions Bot removed the stale label Oct 23, 2025
@Baoyuantop Baoyuantop added the wait for update wait for the author's response in this issue/PR label Dec 10, 2025
@Baoyuantop

Copy link
Copy Markdown
Contributor

Hi @ranxuxin001, following up on the previous review comments. Let us know if you have an update. Thanks!

@apaloleg

Copy link
Copy Markdown

@Baoyuantop Hi! I open new MR #12826 for this issue

@Baoyuantop

Copy link
Copy Markdown
Contributor

Due to a lack of activity, this PR is closed for now. We will continue working on this issue in #12826. Thank you for your contribution @ranxuxin001 .

@Baoyuantop Baoyuantop closed this Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wait for update wait for the author's response in this issue/PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.