Do not instantiate load balance if there is no invokers (#1297)#1376
Merged
chickenlj merged 2 commits intoapache:masterfrom Feb 24, 2018
Merged
Do not instantiate load balance if there is no invokers (#1297)#1376chickenlj merged 2 commits intoapache:masterfrom
chickenlj merged 2 commits intoapache:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1376 +/- ##
==========================================
- Coverage 32.58% 32.41% -0.18%
==========================================
Files 691 691
Lines 34537 34539 +2
Branches 6812 6813 +1
==========================================
- Hits 11255 11196 -59
- Misses 21356 21410 +54
- Partials 1926 1933 +7
Continue to review full report at Codecov.
|
chickenlj
reviewed
Feb 22, 2018
| .getMethodParameter(invocation.getMethodName(), Constants.LOADBALANCE_KEY, Constants.DEFAULT_LOADBALANCE)); | ||
| } else { | ||
| loadbalance = ExtensionLoader.getExtensionLoader(LoadBalance.class).getExtension(Constants.DEFAULT_LOADBALANCE); | ||
| loadbalance = null; |
Contributor
There was a problem hiding this comment.
How about remove this else statement and move loadbalance = ExtensionLoader.getExtensionLoader(LoadBalance.class).getExtension(Constants.DEFAULT_LOADBALANCE); to doSelect()?
Contributor
Author
There was a problem hiding this comment.
You mean something like:
private Invoker<T> doselect(LoadBalance loadbalance, Invocation invocation, List<Invoker<T>> invokers, List<Invoker<T>> selected) throws RpcException {
if (invokers == null || invokers.isEmpty())
return null;
if (invokers.size() == 1)
return invokers.get(0);
// If we only have two invokers, use round-robin instead.
if (invokers.size() == 2 && selected != null && !selected.isEmpty()) {
return selected.get(0) == invokers.get(0) ? invokers.get(1) : invokers.get(0);
}
if (loadbalance == null) {
loadbalance = ExtensionLoader.getExtensionLoader(LoadBalance.class).getExtension(Constants.DEFAULT_LOADBALANCE);
}
Invoker<T> invoker = loadbalance.select(invokers, getUrl(), invocation);
//If the `invoker` is in the `selected` or invoker is unavailable && availablecheck is true, reselect.
if ((selected != null && selected.contains(invoker))
|| (!invoker.isAvailable() && getUrl() != null && availablecheck)) {
try {
Invoker<T> rinvoker = reselect(loadbalance, invocation, invokers, selected, availablecheck);
if (rinvoker != null) {
invoker = rinvoker;
} else {
//Check the index of current selected invoker, if it's not the last one, choose the one at index+1.
int index = invokers.indexOf(invoker);
try {
//Avoid collision
invoker = index < invokers.size() - 1 ? invokers.get(index + 1) : invoker;
} catch (Exception e) {
logger.warn(e.getMessage() + " may because invokers list dynamic change, ignore.", e);
}
}
} catch (Throwable t) {
logger.error("cluster reselect fail reason is :" + t.getMessage() + " if can not solve, you can set cluster.availablecheck=false in url", t);
}
}
return invoker;
}
public Result invoke(final Invocation invocation) throws RpcException {
checkWhetherDestroyed();
LoadBalance loadbalance = null;
List<Invoker<T>> invokers = list(invocation);
if (invokers != null && !invokers.isEmpty()) {
loadbalance = ExtensionLoader.getExtensionLoader(LoadBalance.class).getExtension(invokers.get(0).getUrl()
.getMethodParameter(invocation.getMethodName(), Constants.LOADBALANCE_KEY, Constants.DEFAULT_LOADBALANCE));
}
RpcUtils.attachInvocationIdIfAsync(getUrl(), invocation);
return doInvoke(invocation, invokers, loadbalance);
}
If that, I'm not sure why we need to expose loadbalance to doInvoke and select, unless we allow subclass to pass a custom loadbalance to select. Thank you so much!
Contributor
There was a problem hiding this comment.
checkInvokers(copyinvokers, invocation); should always be called every time before we actually invoke.
But what if developers extending AbstractClusterInvoker don't do this check? The loadbalance will be null.
zonghaishang
pushed a commit
to zonghaishang/dubbo
that referenced
this pull request
Feb 26, 2018
* @reference support annotate on annotation type * Fixes apache#1303 TimeUnit conversion error * Fixes apache#1289, use bind_port as mapping key * Fixes apache#1313, remove destroy check in Registry. * checkout .travis.yml from origin/master * Fix hessian2 serialized short, byte is converted to int bug (apache#1232) * Fix hessian2 serialized short, byte is converted to int bug * Fix hessian2 serialized short, byte is converted to int bug * adapt jdk1.5+ * fixed travis-ci failed because of test cases. (apache#1370) * Merge pull request apache#1377, remove redundant arguments for StatItem.isAllowable() * Merge pull request apache#1378, replace StringBuider with simple string concatenation in log. * Merge pull request apache#1375, remove unnecessary boxing. Fixes apache#1245 * Merge pull request apache#1331, add optional parameter to support hessian protocol method overload and request protocol version. * Merge pull request apache#1376, do not instantiate load balance if there is no invokers Fixes apache#1297 * Merge pull request apache#1384, fix build string bug. * Merge pull request apache#1040, refactor: replace some deprecated methods related with jedis. * Merge pull request apache#1242, remove redundant null check. fixes apache#1231
zonghaishang
pushed a commit
to zonghaishang/dubbo
that referenced
this pull request
Feb 26, 2018
* @reference support annotate on annotation type * Fixes apache#1303 TimeUnit conversion error * Fixes apache#1289, use bind_port as mapping key * Fixes apache#1313, remove destroy check in Registry. * checkout .travis.yml from origin/master * Fix hessian2 serialized short, byte is converted to int bug (apache#1232) * Fix hessian2 serialized short, byte is converted to int bug * Fix hessian2 serialized short, byte is converted to int bug * adapt jdk1.5+ * fixed travis-ci failed because of test cases. (apache#1370) * Merge pull request apache#1377, remove redundant arguments for StatItem.isAllowable() * Merge pull request apache#1378, replace StringBuider with simple string concatenation in log. * Merge pull request apache#1375, remove unnecessary boxing. Fixes apache#1245 * Merge pull request apache#1331, add optional parameter to support hessian protocol method overload and request protocol version. * Merge pull request apache#1376, do not instantiate load balance if there is no invokers Fixes apache#1297 * Merge pull request apache#1384, fix build string bug. * Merge pull request apache#1040, refactor: replace some deprecated methods related with jedis. * Merge pull request apache#1242, remove redundant null check. fixes apache#1231 * Change Mailing list address * Fixed apache#1398, revert bugs introduced from apache#1375 * Fix time unit problem in UT * Fix time unit problem related with FutureAdapter in UT
zonghaishang
pushed a commit
to zonghaishang/dubbo
that referenced
this pull request
Mar 2, 2018
* @reference support annotate on annotation type * Fixes apache#1303 TimeUnit conversion error * Fixes apache#1289, use bind_port as mapping key * Fixes apache#1313, remove destroy check in Registry. * checkout .travis.yml from origin/master * Fix hessian2 serialized short, byte is converted to int bug (apache#1232) * Fix hessian2 serialized short, byte is converted to int bug * Fix hessian2 serialized short, byte is converted to int bug * adapt jdk1.5+ * fixed travis-ci failed because of test cases. (apache#1370) * Merge pull request apache#1377, remove redundant arguments for StatItem.isAllowable() * Merge pull request apache#1378, replace StringBuider with simple string concatenation in log. * Merge pull request apache#1375, remove unnecessary boxing. Fixes apache#1245 * Merge pull request apache#1331, add optional parameter to support hessian protocol method overload and request protocol version. * Merge pull request apache#1376, do not instantiate load balance if there is no invokers Fixes apache#1297 * Merge pull request apache#1384, fix build string bug. * Merge pull request apache#1040, refactor: replace some deprecated methods related with jedis. * Merge pull request apache#1242, remove redundant null check. fixes apache#1231 * Change Mailing list address * Fixed apache#1398, revert bugs introduced from apache#1375 * Fix time unit problem in UT * Fix time unit problem related with FutureAdapter in UT * Fix time unit problem related with FutureAdapter in UT * Merge pull request apache#1391, fix typo of method name in qos module. * fix hessian lite test case fail bug (apache#1394) * fix hessian lite test case fail bug * update test * remove ignore * Fix time unit problem related with FutureAdapter in UT * revert file * fix number type is lost in yaml config file (apache#1401) * apache#1399 fi * update test * update readme to add some details (apache#1403) * update readme to add some details update readme to add some details * delete duplicated words delete duplicated words * update README format * apache#1411: Locale deserialize 'zh-hant_CN'
xpylq
pushed a commit
to xpylq/dubbo
that referenced
this pull request
Mar 5, 2018
* remotes/upstream/master: (226 commits) clean up imports for CacheTest [Dubbo-apache#1362] cache provider always lru cache (apache#1396) Remove author info and add apache license Fix "promoteTransitiveDependencies=false" of maven-shade-plugin apache#1411: Locale deserialize 'zh-hant_CN' update README format update readme to add some details (apache#1403) fix number type is lost in yaml config file (apache#1401) fix hessian lite test case fail bug (apache#1394) Merge pull request apache#1391, fix typo of method name in qos module. Fix time unit problem related with FutureAdapter in UT Fix time unit problem related with FutureAdapter in UT Fix time unit problem in UT Fixed apache#1398, revert bugs introduced from apache#1375 Change Mailing list address Merge pull request apache#1242, remove redundant null check. Merge pull request apache#1040, refactor: replace some deprecated methods related with jedis. Merge pull request apache#1384, fix build string bug. Merge pull request apache#1376, do not instantiate load balance if there is no invokers Merge pull request apache#1331, add optional parameter to support hessian protocol method overload and request protocol version. ...
rolandhe
pushed a commit
to rolandhe/dubbo
that referenced
this pull request
Sep 9, 2019
…ere is no invokers Fixes apache#1297
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, we'll instantiate the load balance with a default instance if no invokers found. However, we don't need to use it.
Closes #1297