Condition route v3.1 #14356
Condition route v3.1 #14356
Conversation
This reverts commit 57b3e6a.
| @@ -125,7 +125,7 @@ public final BitList<Invoker<T>> route( | |||
|
|
|||
| routeResult = doRoute(invokers, url, invocation, needToPrintMessage, nodeHolder, messageHolder); | |||
| if (routeResult != invokers) { | |||
| routeResult = invokers.and(routeResult); | |||
| routeResult = routeResult.and(invokers); | |||
There was a problem hiding this comment.
In the new route rule, I will determine if the invoker object's memory address has changed to determine if it is a valid route. So this order change is necessary.
There was a problem hiding this comment.
replace with invokers.clone().and(routeResult)
| if (needToPrintMessage) { | ||
| resultMessage.append(messageHolder.get()); | ||
| } | ||
|
|
||
| if (needToPrintMessage) { | ||
| messageHolder.set(resultMessage.toString()); | ||
| } |
There was a problem hiding this comment.
I've changed it here.
| || routerRule.getVersion() != null && routerRule.getVersion().startsWith(RULE_VERSION_V31)) { | ||
| boolean trafficDisable = false; | ||
| for (MultiDestConditionRouter<T> multiDestConditionRouter : multiDestConditionRouters) { | ||
| routeResult = multiDestConditionRouter.route(invokers, url, invocation, needToPrintMessage, nodeHolder); |
There was a problem hiding this comment.
Should clone invokers here first
There was a problem hiding this comment.
I don't know if cloning is necessary here; the v3.0 logic just passes the filtering on invokers itself.
There was a problem hiding this comment.
In the route method, the value in invokers is not modified directly, and there are three cases:
1.Return invokers directly
2. return BItlist.empty
3. The invokers will be cloned and modified then return
so I don't think it is necessary to clone first here, just keep the same with v3.0
| return moduleModel | ||
| .getExtensionLoader(ConditionMatcherFactory.class) | ||
| .getExtension("param") |
There was a problem hiding this comment.
Hard code here. If there only exist one implementation, use it directly
There was a problem hiding this comment.
Here I just kept the same code with v3.0.Let me think about whether there's room for optimization.
| "Invalid condition config version number.", | ||
| "", | ||
| "Ignore this configuration. Only " + RULE_VERSION_V31 + " and below are supported in this release"); | ||
| rule = ConditionRouterRule.parseFromMap(map); |
There was a problem hiding this comment.
Should we continue with parsing when found configVersion not match?
There was a problem hiding this comment.
An alternative is to simply create a new ConditionRouterRule object.Otherwise, the rule property is null. Even though there may be a null test, I think it's best to try not to generate null.
There was a problem hiding this comment.
Just set it to null and add a unit test
|
What is the purpose of the change
Added multi-address and priority support
Brief changelog
Detailed design documentation is available here #14344
Verifying this change
Checklist