You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org> on 2023/03/10 04:00:42 UTC

[GitHub] [apisix] monkeyDluffy6017 opened a new pull request, #9047: change: change the default router from radixtree uri to radixtree hos…

monkeyDluffy6017 opened a new pull request, #9047:
URL: https://github.com/apache/apisix/pull/9047

   ### Description
   
   Update the default HTTP router from radixtree_uri to radixtree_host_uri .
   
   > So when Apache APISIX uses radixtree_uri as the default HTTP router,
   there is a confusing scenario (even we can say it's a bug): Let's say
   we have two routes, the first one requires the host matches
   *.[example.com](http://example.com/), and the URI path is /anything, And the second one needs
   the host matches [foo.example.com](http://foo.example.com/) exactly, and the URI path is also
   /anything. In such a case, if we send an API request, we may hit the
   first route, which is counterintuitive.
   
   We also have a lot of voices from the community that users have been
   in trouble since they think Apache APISIX will consider the HTTP host
   match by default. However, it's not the case. So I'm here to propose
   changing the default HTTP router to radixtree_host_uri.
   
   Fixes #8354
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [ ] I have added tests corresponding to this change
   - [x] I have updated the documentation to reflect this change
   - [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
   


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9047: change: change the default router from radixtree uri to radixtree hos…

Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9047:
URL: https://github.com/apache/apisix/pull/9047#discussion_r1137987478


##########
t/router/radixtree-method.t:
##########
@@ -16,9 +16,20 @@
 #
 use t::APISIX 'no_plan';
 
+our $yaml_config = <<_EOC_;
+apisix:
+    node_listen: 1984
+    router:
+        http: 'radixtree_uri'

Review Comment:
   I was wondering if it would be better to specify the routing engine for these test cases on router, so that the default engine is changed and the test cases are not affected



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander merged pull request #9047: change: change the default router from radixtree uri to radixtree hos…

Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander merged PR #9047:
URL: https://github.com/apache/apisix/pull/9047


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9047: change: change the default router from radixtree uri to radixtree hos…

Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9047:
URL: https://github.com/apache/apisix/pull/9047#discussion_r1134798422


##########
t/router/radixtree-method.t:
##########
@@ -16,9 +16,20 @@
 #
 use t::APISIX 'no_plan';
 
+our $yaml_config = <<_EOC_;
+apisix:
+    node_listen: 1984
+    router:
+        http: 'radixtree_uri'

Review Comment:
   It can pass, should we use the default router here?



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] monkeyDluffy6017 commented on pull request #9047: change: change the default router from radixtree uri to radixtree hos…

Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #9047:
URL: https://github.com/apache/apisix/pull/9047#issuecomment-1465438820

   > 
   
   @spacewander do you mean remove the config like https://github.com/apache/apisix/blob/master/t/router/radixtree-host-uri.t#L29?


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on a diff in pull request #9047: change: change the default router from radixtree uri to radixtree hos…

Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on code in PR #9047:
URL: https://github.com/apache/apisix/pull/9047#discussion_r1134793896


##########
t/router/radixtree-method.t:
##########
@@ -16,9 +16,20 @@
 #
 use t::APISIX 'no_plan';
 
+our $yaml_config = <<_EOC_;
+apisix:
+    node_listen: 1984
+    router:
+        http: 'radixtree_uri'

Review Comment:
   Can't this test pass with the default router?



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on a diff in pull request #9047: change: change the default router from radixtree uri to radixtree hos…

Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on code in PR #9047:
URL: https://github.com/apache/apisix/pull/9047#discussion_r1133365848


##########
docs/en/latest/terminology/router.md:
##########
@@ -38,13 +38,19 @@ A Router can have the following configurations:
 
 - `apisix.router.http`: The HTTP request route. It can take the following values:
 
-  - `radixtree_uri`: (Default) Only use the `uri` as the primary index. To learn more about the support for full and deep prefix matching, check [How to use router-radixtree](../router-radixtree.md).
+  - `radixtree_uri`: Only use the `uri` as the primary index. To learn more about the support for full and deep prefix matching, check [How to use router-radixtree](../router-radixtree.md).
     - `Absolute match`: Match completely with the given `uri` (`/foo/bar`, `/foo/glo`).
     - `Prefix match`: Match with the given prefix. Use `*` to represent the given `uri` for prefix matching. For example, `/foo*` can match with `/foo/`, `/foo/a` and `/foo/b`.
     - `match priority`: First try an absolute match, if it didn't match, try prefix matching.
     - `Any filter attribute`: This allows you to specify any Nginx built-in variable as a filter, such as URL request parameters, request headers, and cookies.
   - `radixtree_uri_with_parameter`: Like `radixtree_uri` but also supports parameter match.
-  - `radixtree_host_uri`: Matches both host and URI of the request. Use `host + uri` as the primary index (based on the `radixtree` engine).
+  - `radixtree_host_uri`: (Default) Matches both host and URI of the request. Use `host + uri` as the primary index (based on the `radixtree` engine).
+
+:::note
+
+In version 3.2 and earlier, APISIX used `radixtree_uri` as the default Router. `radixtree_uri` has better performance than `radixtree_host_uri`, so if you have higher performance requirements and can live with the fact that `radixtree_uri` only matches uri, consider continuing to use `radixtree_uri` as the default Router.

Review Comment:
   Only use the `uri` as the primary index, not only matches uri



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on a diff in pull request #9047: change: change the default router from radixtree uri to radixtree hos…

Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on code in PR #9047:
URL: https://github.com/apache/apisix/pull/9047#discussion_r1137976825


##########
t/router/radixtree-method.t:
##########
@@ -16,9 +16,20 @@
 #
 use t::APISIX 'no_plan';
 
+our $yaml_config = <<_EOC_;
+apisix:
+    node_listen: 1984
+    router:
+        http: 'radixtree_uri'

Review Comment:
   Yes, it would be better. Adding special configuration here makes it like a special router is required.



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on a diff in pull request #9047: change: change the default router from radixtree uri to radixtree hos…

Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on code in PR #9047:
URL: https://github.com/apache/apisix/pull/9047#discussion_r1139579567


##########
t/router/radixtree-method.t:
##########
@@ -16,9 +16,20 @@
 #
 use t::APISIX 'no_plan';
 
+our $yaml_config = <<_EOC_;
+apisix:
+    node_listen: 1984
+    router:
+        http: 'radixtree_uri'

Review Comment:
   It's fine if you want to keep it.



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9047: change: change the default router from radixtree uri to radixtree hos…

Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9047:
URL: https://github.com/apache/apisix/pull/9047#discussion_r1133477378


##########
docs/en/latest/terminology/router.md:
##########
@@ -38,13 +38,19 @@ A Router can have the following configurations:
 
 - `apisix.router.http`: The HTTP request route. It can take the following values:
 
-  - `radixtree_uri`: (Default) Only use the `uri` as the primary index. To learn more about the support for full and deep prefix matching, check [How to use router-radixtree](../router-radixtree.md).
+  - `radixtree_uri`: Only use the `uri` as the primary index. To learn more about the support for full and deep prefix matching, check [How to use router-radixtree](../router-radixtree.md).
     - `Absolute match`: Match completely with the given `uri` (`/foo/bar`, `/foo/glo`).
     - `Prefix match`: Match with the given prefix. Use `*` to represent the given `uri` for prefix matching. For example, `/foo*` can match with `/foo/`, `/foo/a` and `/foo/b`.
     - `match priority`: First try an absolute match, if it didn't match, try prefix matching.
     - `Any filter attribute`: This allows you to specify any Nginx built-in variable as a filter, such as URL request parameters, request headers, and cookies.
   - `radixtree_uri_with_parameter`: Like `radixtree_uri` but also supports parameter match.
-  - `radixtree_host_uri`: Matches both host and URI of the request. Use `host + uri` as the primary index (based on the `radixtree` engine).
+  - `radixtree_host_uri`: (Default) Matches both host and URI of the request. Use `host + uri` as the primary index (based on the `radixtree` engine).
+
+:::note
+
+In version 3.2 and earlier, APISIX used `radixtree_uri` as the default Router. `radixtree_uri` has better performance than `radixtree_host_uri`, so if you have higher performance requirements and can live with the fact that `radixtree_uri` only matches uri, consider continuing to use `radixtree_uri` as the default Router.

Review Comment:
   Done



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org