Skip to content

[SCB-2665]Add the load balance governance #3267

Merged
liubao68 merged 2 commits intoapache:masterfrom
lbc97:master
Aug 8, 2022
Merged

[SCB-2665]Add the load balance governance #3267
liubao68 merged 2 commits intoapache:masterfrom
lbc97:master

Conversation

@lbc97
Copy link
Copy Markdown
Contributor

@lbc97 lbc97 commented Aug 5, 2022

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a JIRA issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [SCB-XXX] Fixes bug in ApproximateQuantiles, where you replace SCB-XXX with the appropriate JIRA issue.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install -Pit to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

Comment on lines +29 to +34
LoadBalanceDecorateCheckedSupplier<Object> ds = LoadBalanceDecorators.ofCheckedSupplier(()->"test");
GovernanceRequest request = new GovernanceRequest();
request.setUri("/loadrandom");
request.setServiceName("RandomTest");
LoadBalance loadBalance = loadBalanceHandler.getActuator(request);
Assert.assertEquals("Random",ds.withFaultInjection(loadBalance));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think for load balance do not need this complicity.

for example

    GovernanceRequest request = new GovernanceRequest();
    request.setUri("/loadrandom");
    request.setServiceName("RandomTest");
    LoadBalance loadBalance = loadBalanceHandler.getActuator(request);
    Assert.assertEquals("Random",loadBalance.getRule());

This is quit enough

Comment on lines +18 to +21
servicecomb:
loadbalance:
rule: |
loadrule: Random
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

servicecomb:
  loadbalance:
    XXXXXXMatch: |
      rule: Random

Use this definition and should define matchGroup for XXXXXXMatch

Comment on lines +18 to +21
package org.apache.servicecomb.governance.policy;

public class LoadBalancerPolicy extends AbstractPolicy {
private String loadrule;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

change to rule

Comment on lines +39 to +41
if (key.equals("rule")) {
return true;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

delete this

Comment on lines +20 to +31
public abstract class AbstractLoadbalance implements LoadBalance {
protected String loadrule;

public AbstractLoadbalance(String loadrule) {
this.loadrule = loadrule;
}

@Override
public String choose() {
return choose(loadrule);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

delete this class

Comment on lines +24 to +28
static String decorateCheckedSupplier(LoadBalance loadBalance) {
return loadBalance.choose();
}

public String choose();

public String choose(String type);

int getOrder();

String getName();
Copy link
Copy Markdown
Contributor

@liubao68 liubao68 Aug 6, 2022

Choose a reason for hiding this comment

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

only need one method getRule which return rule in LoadBalancePolicy

Comment on lines +22 to +39
public interface LoadBalanceDecorators {
static <T> LoadBalanceDecorateCheckedSupplier<T> ofCheckedSupplier(CheckedFunction0<T> supplier) {
return new LoadBalanceDecorateCheckedSupplier<>(supplier);
}

class LoadBalanceDecorateCheckedSupplier<T> {

private CheckedFunction0<T> supplier;

protected LoadBalanceDecorateCheckedSupplier(CheckedFunction0<T> supplier) {
this.supplier = supplier;
}

public String withFaultInjection(LoadBalance loadBalance) {
return LoadBalance.decorateCheckedSupplier(loadBalance);
}

public T get() throws Throwable {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do not need this class

Comment on lines +22 to +28
public class LoadBalanceUtil {

public static LoadBalance getLoadBalance(String key, LoadBalancerPolicy policy) {
LoadBalance loadBalance = null;
if (LoadBalanceConst.TYPE_RANDOM.equals(policy.getLoadrule())) {
loadBalance = new RandomBalancer(LoadBalanceConst.TYPE_RANDOM);
} else if (LoadBalanceConst.TYPE_ROUNDROBIN.equals(policy.getLoadrule())) {
loadBalance = new RoundRobinBalancer(LoadBalanceConst.TYPE_ROUNDROBIN);
}
return loadBalance;
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do not need this

Comment on lines +3 to +23
public class RandomBalancer extends AbstractLoadbalance implements LoadBalance{

public RandomBalancer(String loadrule) {
super(loadrule);
}

@Override
public String choose(String loadrule) {
return loadrule;
}

@Override
public int getOrder() {
return 100;
}

@Override
public String getName() {
return LoadBalanceConst.TYPE_RANDOM;
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do not need this

Comment on lines +4 to +19

public RoundRobinBalancer(String loadrule) {
super(loadrule);
}

@Override
public String choose(String loadrule) {
return loadrule;
}

@Override
public int getOrder() {
return 100;
}

@Override
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

delete this class


@Override
protected LoadBalance createProcessor(String key, GovernanceRequest governanceRequest, LoadBalancerPolicy policy) {
return LoadBalanceUtil.getLoadBalance(key, policy);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

return new LoadBalanceImpl()

class LoadBalanceImpl implements LoadBalance {
   @Override
   public String getRule() {
     return policy.getRule();
  }
}

Comment on lines +19 to +29
@Override
public int getOrder() {
return 100;
}

@Override
public String getName() {
return null;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This two methods never read, why add them?

package org.apache.servicecomb.loadbanlance;

public class LoadBalanceImpl implements LoadBalance {
private String rule;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should be final

Comment on lines +6 to +12
public String getRule() {
return rule;
}

public void setRule(String rule) {
this.rule = rule;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

put methods definition after constructor

Comment on lines +22 to +28
public class LoadBalanceUtil {

public static LoadBalance getLoadBalance(String key, LoadBalancerPolicy policy) {
LoadBalance loadBalance = new LoadBalanceImpl(policy.getRule());
return loadBalance;
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

delete this class and inline code

Comment on lines +26 to +28
int getOrder();

String getName();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

these two methods never read, why add them?

@@ -0,0 +1,28 @@
package org.apache.servicecomb.loadbanlance;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add license

@@ -0,0 +1,43 @@
package org.apache.servicecomb.governance;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add license

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #3267 (507e1b4) into master (31ef85f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 507e1b4 differs from pull request most recent head e6a5b75. Consider uploading reports for the commit e6a5b75 to get more accurate results

@@             Coverage Diff              @@
##             master    #3267      +/-   ##
============================================
+ Coverage     77.44%   77.45%   +0.01%     
  Complexity     1439     1439              
============================================
  Files          1641     1646       +5     
  Lines         43575    43596      +21     
  Branches       3665     3665              
============================================
+ Hits          33746    33767      +21     
  Misses         8309     8309              
  Partials       1520     1520              
Impacted Files Coverage Δ
...ervicecomb/governance/GovernanceConfiguration.java 100.00% <100.00%> (ø)
...icecomb/governance/handler/LoadBalanceHandler.java 100.00% <100.00%> (ø)
...vicecomb/governance/policy/LoadBalancerPolicy.java 100.00% <100.00%> (ø)
...b/governance/properties/LoadBalanceProperties.java 100.00% <100.00%> (ø)
...g/apache/servicecomb/loadbanlance/LoadBalance.java 100.00% <100.00%> (ø)
...ache/servicecomb/loadbanlance/LoadBalanceImpl.java 100.00% <100.00%> (ø)
.../servicecomb/registry/discovery/DiscoveryTree.java 96.49% <0.00%> (-3.51%) ⬇️
...thentication/consumer/RSAConsumerTokenManager.java 77.41% <0.00%> (+6.45%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@lbc97 lbc97 changed the title add loadbalance governance [SCB-2665]Add the load balancing function Aug 8, 2022
@liubao68 liubao68 changed the title [SCB-2665]Add the load balancing function [SCB-2665]Add the load balance governance Aug 8, 2022
@liubao68 liubao68 merged commit 493fcdc into apache:master Aug 8, 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.

3 participants