Skip to content

Conversation

@liubao68
Copy link
Contributor

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 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.

@liubao68 liubao68 changed the title [SBC-870]refractor to using custom RuleExt, not using Robin IRule. [SBC-870]refactor to using custom RuleExt, not using Robin IRule. Aug 24, 2018
@liubao68
Copy link
Contributor Author

  1. refactor to RuleExt
  2. fix a bug required=true that int will have default value
  3. This commit is a initial commit, I am going to add more tests

List<AtomicInteger> stats = new ArrayList<>(servers.size());
int totalWeights = 0;
boolean needRandom = false;
for (ServiceCombServer server : servers) {
Copy link
Contributor

Choose a reason for hiding this comment

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

每次都进行重新计算,如果server很多的时候,会影响性能吧

Copy link
Contributor Author

@liubao68 liubao68 Aug 25, 2018

Choose a reason for hiding this comment

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

权重算法就是这样的。对我们的场景,这个计算量很少,以及过滤掉不可用实例了。 即使200实例,这个算法复杂度也很低。比原来的Robin的WeightedResonseTimeRule要快,它的计算量更多。而且现在还省去了timer的后台计算。 现在是简单权重算法,来换取可靠性,不追求权重的精确性。

}
this.policy = policy;
this.strategy = strategy;
LoadBalancer loadBalancer = getOrCreateLoadBalancer(invocation);
Copy link
Contributor

Choose a reason for hiding this comment

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

better to make loadBalancer statless
1.invoke discoveryTree before getOrCreateLoadBalancer
2.getOrCreateLoadBalancer
3.send or sendWithRetry with server list
chooseServer with server list
create RetryLoadBalancer instance with server list

String strategy = Configuration.INSTANCE.getRuleStrategyName(invocation.getMicroserviceName());
boolean isRuleNotChanged = isEqual(policy, this.policy) && isEqual(strategy, this.strategy);
if (!isRuleNotChanged) {
if (!isEqual(strategy, this.strategy)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

it's better to check if customers being using old policy configuration, and tell them how to switch to new mechanism
check can be done when init, not when handle
currently maybe init method not invoked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@Override
public IRule createLoadBalancerRule(String ruleName) {
public RuleExt createLoadBalancerRule(String ruleName) {
if (RULE_RoundRobin.equals(ruleName)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

support SPI?
and use a map to manage them?

Copy link
Contributor Author

@liubao68 liubao68 Aug 25, 2018

Choose a reason for hiding this comment

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

Now the extension point is ExtensionsFactory, not RuleExt. Maybe we can work RuleExt based on SPI in future, now no need to give so many extension points.

@liubao68 liubao68 closed this Aug 25, 2018
@liubao68 liubao68 reopened this Aug 25, 2018
List<AtomicInteger> stats = new ArrayList<>(servers.size());
int totalWeights = 0;
boolean needRandom = false;
for (ServiceCombServer server : servers) {
Copy link
Contributor

Choose a reason for hiding this comment

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

calc weight for every invocation is not a good idea

boolean needRandom = false;
for (ServiceCombServer server : servers) {
ServiceCombServerStats serverStats = ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(server);
int avgTime = (int) serverStats.getAverageResponseTime();
Copy link
Contributor

Choose a reason for hiding this comment

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

avg result better to be double

@coveralls
Copy link

coveralls commented Aug 25, 2018

Coverage Status

Coverage decreased (-0.02%) to 86.156% when pulling 789ab3e on liubao68:empty_param into ea17cca on apache:master.

@liubao68 liubao68 merged commit b9eed06 into apache:master Aug 27, 2018
@liubao68 liubao68 deleted the empty_param branch March 9, 2021 10:51
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.

4 participants