Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adaptive loadbalance #10745

Merged
merged 24 commits into from
Dec 13, 2022
Merged

Adaptive loadbalance #10745

merged 24 commits into from
Dec 13, 2022

Conversation

ningboliu
Copy link
Contributor

@ningboliu ningboliu commented Oct 13, 2022

What is the purpose of the change

issue #10571
adaptive loadbalance

Brief changelog

Verifying this change

Checklist

  • Make sure there is a GitHub_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
  • Add some description to dubbo-website project if you are requesting to add a feature.
  • GitHub Actions works fine on your own branch.
  • If this contribution is large, please follow the Software Donation Guide.

@ningboliu
Copy link
Contributor Author

ningboliu commented Oct 13, 2022

基准测试
环境
Linux version 3.10.0-1127.el7.x86_64
4C8G
java version "11.0.16.1"
网络:内网千兆网络

启动参数:-Xms2048m -Xmx2048m -XX:+UseG1GC

之前写的基准测试有问题,拿到的都是同一LoadBalance,以下数据为更新后的。
2服务器

Benchmark                      Mode  Cnt     Score      Error  Units
LoadBalance.adaptive          thrpt    4  7156.037 ± 9787.735  ops/s
LoadBalance.random            thrpt    4  6475.586 ± 2325.695  ops/s
LoadBalance.roundrobin        thrpt    4  6498.221 ± 1064.650  ops/s
LoadBalance.shortestresponse  thrpt    4  6407.356 ±  560.281  ops/s

3服务器,其中一台cpu 100%

Benchmark                      Mode  Cnt      Score      Error  Units
LoadBalance.adaptive          thrpt    4  15035.626 ± 2484.066  ops/s
LoadBalance.random            thrpt    4  10191.162 ± 2346.887  ops/s
LoadBalance.roundrobin        thrpt    4  10064.456 ± 4554.784  ops/s
LoadBalance.shortestresponse  thrpt    4  10487.329 ± 2498.746  ops/s

@AlbumenJ AlbumenJ changed the base branch from 3.1 to 3.2 October 14, 2022 02:39
/**
* uses a single worker thread operating off an bounded queue
*/
private static final ThreadPoolExecutor executor = new ThreadPoolExecutor(1, 1, 0L,TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(1024),
Copy link
Member

Choose a reason for hiding this comment

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

should be lazy init or use dubbo shared executor

@ningboliu
Copy link
Contributor Author

ningboliu commented Oct 21, 2022

三轮基准测试结果
image

3服务器均空闲
image

Comment on lines 58 to 59
executor = new ThreadPoolExecutor(1, 1, 0L,TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(1024),
new NamedInternalThreadFactory("Dubbo-framework-loadbalance-adaptive", true), new ThreadPoolExecutor.DiscardOldestPolicy());
Copy link
Member

Choose a reason for hiding this comment

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

should not create executor by default when user do not use this

Comment on lines 60 to 61
executor = new ThreadPoolExecutor(1, 1, 0L,TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(1024),
new NamedInternalThreadFactory("Dubbo-framework-loadbalance-adaptive", true), new ThreadPoolExecutor.DiscardOldestPolicy());
Copy link
Member

Choose a reason for hiding this comment

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

Register to org.apache.dubbo.common.resource.GlobalResourcesRepository#registerDisposable

*/
public class AdaptiveMetrics {

private static final ConcurrentMap<String, AdaptiveMetrics> METRICS_STATISTICS = new ConcurrentHashMap<String,
Copy link
Member

Choose a reason for hiding this comment

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

Should not use static field to store status in Dubbo. Use scope bean instead

* @see org.apache.dubbo.rpc.Filter
* @see org.apache.dubbo.rpc.RpcContext
*/
@Activate(group = CONSUMER, order = -200000)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@Activate(group = CONSUMER, order = -200000)
@Activate(group = CONSUMER, order = -200000, value = {"loadbalance:adaptive"})

private AdaptiveMetrics adaptiveMetrics;

@Override
public void setApplicationModel(ApplicationModel scopeModel) {
Copy link
Member

Choose a reason for hiding this comment

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

replace to constructor

public void setApplicationModel(ApplicationModel scopeModel) {
AdaptiveMetrics bean = scopeModel.getBeanFactory().getBean(AdaptiveMetrics.class);
if (bean == null) {
scopeModel.getBeanFactory().registerBean(new AdaptiveMetrics());
Copy link
Member

Choose a reason for hiding this comment

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

Add into ScopeBeanInitializer

Comment on lines 85 to 90
private AdaptiveMetrics getAdaptiveMetricsInstance(){
if (adaptiveMetrics == null) {
adaptiveMetrics = scopeModel.getBeanFactory().getBean(AdaptiveMetrics.class);
}
return adaptiveMetrics;
}
Copy link
Member

Choose a reason for hiding this comment

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

move to constructor

Comment on lines 63 to 70
@Override
public void setApplicationModel(ApplicationModel scopeModel) {
AdaptiveMetrics bean = scopeModel.getBeanFactory().getBean(AdaptiveMetrics.class);
if (bean == null) {
scopeModel.getBeanFactory().registerBean(new AdaptiveMetrics());
}
this.scopeModel = scopeModel;
}
Copy link
Member

Choose a reason for hiding this comment

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

move to constructor

Comment on lines 57 to 59
if (bean == null) {
scopeModel.getBeanFactory().registerBean(new AdaptiveMetrics());
}
Copy link
Member

Choose a reason for hiding this comment

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

use ScopeBeanInitializer

@Override
protected <T> Invoker<T> doSelect(List<Invoker<T>> invokers, URL url, Invocation invocation) {
Invoker invoker = selectByP2C(invokers,url,invocation);
invocation.setAttachment(Constants.ADAPTIVE_LOADBALANCE_ATTACHMENT_KEY,attachmentKey);
Copy link
Member

Choose a reason for hiding this comment

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

use attribute would better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to pass parameters to provider

Comment on lines 99 to 102
private String buildServiceKey(Invoker<?> invoker,Invocation invocation){
URL url = invoker.getUrl();
return url.getAddress() + ":" + invocation.getProtocolServiceKey();
}
Copy link
Member

Choose a reason for hiding this comment

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

cache this in invocation attribute

Comment on lines 105 to 114
URL url = invoker.getUrl();
Object countdown = RpcContext.getClientAttachment().getObjectAttachment(TIME_COUNTDOWN_KEY);
int timeout;
if (countdown == null) {
timeout = (int) RpcUtils.getTimeout(url, invocation.getMethodName(), RpcContext.getClientAttachment(), DEFAULT_TIMEOUT);
} else {
TimeoutCountDown timeoutCountDown = (TimeoutCountDown) countdown;
timeout = (int) timeoutCountDown.timeRemaining(TimeUnit.MILLISECONDS);
}
return timeout;
Copy link
Member

Choose a reason for hiding this comment

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

why read current invocation's timeout?

private <T> Invoker<T> chooseLowLoadInvoker(Invoker<T> invoker1,Invoker<T> invoker2,Invocation invocation){
int weight1 = getWeight(invoker1, invocation);
int weight2 = getWeight(invoker2, invocation);
int timeout1 = getTimeout(invoker2, invocation);
Copy link
Member

Choose a reason for hiding this comment

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

Are there some spell mistakes?

* @see org.apache.dubbo.rpc.RpcContext
*/
@Activate(group = CONSUMER, order = -200000, value = {"loadbalance:adaptive"})
public class AdaptiveLoadBalanceFilter implements ClusterFilter, ClusterFilter.Listener {
Copy link
Member

Choose a reason for hiding this comment

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

Move this filter to implement Filter would be better. The implementation are related with the actual provider address.

Comment on lines 80 to 97
private String buildServiceKey(Invocation invocation){
StringBuilder sb = new StringBuilder(128);
sb.append(invocation.getInvoker().getUrl().getAddress()).append(":").append(invocation.getProtocolServiceKey());
return sb.toString();
//return url.getAddress() + ProtocolUtils.serviceKey(url.getPort(), url.getPath(), url.getVersion(), url.getGroup());
}

private String getServiceKey(Invocation invocation){

String key = (String) invocation.getAttributes().get(invocation.getInvoker());
if (StringUtils.isNotEmpty(key)){
return key;
}

key = buildServiceKey(invocation);
invocation.getAttributes().put(invocation.getInvoker(),key);
return key;
}
Copy link
Member

Choose a reason for hiding this comment

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

Will this key be reused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

used 2 times

@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2022

Codecov Report

Merging #10745 (a65dbde) into 3.2 (9508acd) will decrease coverage by 0.01%.
The diff coverage is 57.14%.

@@             Coverage Diff              @@
##                3.2   #10745      +/-   ##
============================================
- Coverage     64.90%   64.88%   -0.02%     
  Complexity       14       14              
============================================
  Files          1468     1471       +3     
  Lines         61031    61245     +214     
  Branches       8944     8983      +39     
============================================
+ Hits          39614    39741     +127     
- Misses        17233    17296      +63     
- Partials       4184     4208      +24     
Impacted Files Coverage Δ
...he/dubbo/rpc/filter/AdaptiveLoadBalanceFilter.java 0.00% <0.00%> (ø)
.../apache/dubbo/rpc/filter/ProfilerServerFilter.java 55.55% <33.33%> (-5.56%) ⬇️
...o/rpc/cluster/loadbalance/AdaptiveLoadBalance.java 88.00% <88.00%> (ø)
...ain/java/org/apache/dubbo/rpc/AdaptiveMetrics.java 89.79% <89.79%> (ø)
...ubbo/rpc/cluster/ClusterScopeModelInitializer.java 100.00% <100.00%> (ø)
...che/dubbo/config/utils/DefaultConfigValidator.java 86.36% <0.00%> (-9.10%) ⬇️
...e/dubbo/remoting/transport/netty/NettyChannel.java 52.27% <0.00%> (-7.96%) ⬇️
.../rpc/cluster/router/mock/MockInvokersSelector.java 58.97% <0.00%> (-5.13%) ⬇️
.../apache/dubbo/remoting/transport/AbstractPeer.java 67.39% <0.00%> (-4.35%) ⬇️
.../cluster/filter/support/ConsumerContextFilter.java 61.22% <0.00%> (-4.00%) ⬇️
... and 33 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines 162 to 164
System.out.println(sumInvoker1);
System.out.println(sumInvoker2);
System.out.println(sumInvoker5);
Copy link
Member

Choose a reason for hiding this comment

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

Print here is useless

int expectWeightValue = loop / totalWeight;
int maxDeviation = expectWeightValue * 2;
double beta = 0.5;
//这个估算值并不准确
Copy link
Member

Choose a reason for hiding this comment

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

comment in English

Comment on lines +71 to +73
System.out.println(sumInvoker1);
System.out.println(sumInvoker2);
System.out.println(sumInvoker3);
Copy link
Member

Choose a reason for hiding this comment

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

Print is useless in unit test

Copy link
Member

@AlbumenJ AlbumenJ left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarcloud
Copy link

sonarcloud bot commented Dec 13, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

28.4% 28.4% Coverage
0.0% 0.0% Duplication

@AlbumenJ AlbumenJ merged commit d171605 into apache:3.2 Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants