Re: [PR] fix: split global rules phase execution for client-control compatibility [apisix]
nic-6443 merged PR #13345: URL: https://github.com/apache/apisix/pull/13345 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: split global rules phase execution for client-control compatibility [apisix]
nic-6443 commented on code in PR #13345: URL: https://github.com/apache/apisix/pull/13345#discussion_r3215995665 ## apisix/init.lua: ## Review Comment: Done — split the no-route path into explicit rewrite + access calls, and removed the nil handling from `run_global_rules()` entirely. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: split global rules phase execution for client-control compatibility [apisix]
membphis commented on code in PR #13345: URL: https://github.com/apache/apisix/pull/13345#discussion_r3215985303 ## apisix/init.lua: ## Review Comment: agree -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: split global rules phase execution for client-control compatibility [apisix]
nic-chen commented on code in PR #13345: URL: https://github.com/apache/apisix/pull/13345#discussion_r3215934401 ## apisix/init.lua: ## Review Comment: It is recommended to process all phases with nil names by calling rewrite and access separately, which would make things clearer. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: split global rules phase execution for client-control compatibility [apisix]
nic-6443 commented on code in PR #13345:
URL: https://github.com/apache/apisix/pull/13345#discussion_r3212612774
##
apisix/init.lua:
##
@@ -850,6 +853,7 @@ function _M.http_access_phase()
plugin.run_plugin(phase, api_ctx.plugins, api_ctx)
end
end
+plugin.run_global_rules(api_ctx, global_rules, conf_version, "access")
plugin.run_plugin("access", plugins, api_ctx)
Review Comment:
The expensive part (merging global rules) is already LRU-cached. The
`filter()` call is lightweight — it just iterates through plugins and builds a
priority-sorted list. The overhead of calling it twice is negligible compared
to the actual plugin execution. Adding a cache for the filtered list would
introduce complexity (cache invalidation on plugin changes) without meaningful
perf benefit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] fix: split global rules phase execution for client-control compatibility [apisix]
Copilot commented on code in PR #13345:
URL: https://github.com/apache/apisix/pull/13345#discussion_r3212599860
##
apisix/init.lua:
##
@@ -850,6 +853,7 @@ function _M.http_access_phase()
plugin.run_plugin(phase, api_ctx.plugins, api_ctx)
end
end
+plugin.run_global_rules(api_ctx, global_rules, conf_version, "access")
plugin.run_plugin("access", plugins, api_ctx)
Review Comment:
Splitting global rules into separate rewrite/access calls means
`plugin.run_global_rules()` now does the merge+`plugin.filter()` work twice per
request (once per phase). Since the merged dummy global rule is already
LRU-cached by `conf_version`, consider caching the filtered global-rule plugin
list (or dummy_global_rule) on `api_ctx` after the rewrite call and reusing it
for the access call to avoid extra allocations and filtering in the hot path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
