Skip to content

Commit

Permalink
loadbalancer-experimental: Unify the Host.weight() methods (#3146)
Browse files Browse the repository at this point in the history
Motivation:

We have a few Host related weight methods, but they have slightly
different names. Worse yet, we don't use them properly when
adjusting weights.

Modifications:

Rename the LoadBalancerObserver.Host.loadBalancingWeight() to
simply .weight()
  • Loading branch information
bryce-anderson authored Dec 19, 2024
1 parent e49939f commit 5fb80d0
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public Addr address() {
}

@Override
public double loadBalancingWeight() {
public double weight() {
return 1;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,14 @@ private void addToResults(int groupProbability, List<H> results) {
// instead to all receive an equal portion of the groups weight.
double weight = ((double) groupProbability) / hosts.size();
for (H host : hosts) {
host.loadBalancingWeight(weight);
host.weight(weight);
results.add(host);
}
} else {
double scalingFactor = groupProbability / groupTotalWeight;
for (H host : hosts) {
double hostWeight = host.loadBalancingWeight() * scalingFactor;
host.loadBalancingWeight(hostWeight);
double hostWeight = host.weight() * scalingFactor;
host.weight(hostWeight);
if (hostWeight > 0) {
results.add(host);
}
Expand All @@ -152,7 +152,7 @@ private void addToResults(int groupProbability, List<H> results) {
private static double totalWeight(Iterable<? extends PrioritizedHost> hosts) {
double sum = 0;
for (PrioritizedHost host : hosts) {
sum += host.loadBalancingWeight();
sum += host.weight();
}
return sum;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ private void sequentialUpdateUsedHosts(List<PrioritizedHostImpl<ResolvedAddress,
// We need to reset the load balancing weights before we run the host set through the rest
// of the operations that will transform and consume the load balancing weight.
for (PrioritizedHostImpl<?, ?> host : nextHosts) {
host.loadBalancingWeight(host.serviceDiscoveryWeight());
host.weight(host.serviceDiscoveryWeight());
}
nextHosts = priorityStrategy.prioritize(nextHosts);
nextHosts = subsetter.subset(nextHosts);
Expand Down Expand Up @@ -706,12 +706,12 @@ void serviceDiscoveryWeight(final double weight) {
// Set the weight to use in load balancing. This includes derived weight information such as prioritization
// and is what the host selectors will use when picking hosts.
@Override
public void loadBalancingWeight(final double weight) {
public void weight(final double weight) {
this.loadBalancingWeight = weight;
}

@Override
public double loadBalancingWeight() {
public double weight() {
return loadBalancingWeight;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,4 @@ interface Host<ResolvedAddress, C extends LoadBalancedConnection> extends Listen
* @return true if the host is now in the closed state, false otherwise.
*/
boolean markExpired();

/**
* The weight of the host, relative to the weights of associated hosts.
* @return the relative weight of the host.
*/
default double weight() {
return 1.0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,6 @@ interface Host {
* The weight of the host, relative to the weights of associated hosts as used for load balancing.
* @return the relative weight of the host.
*/
double loadBalancingWeight();
double weight();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ interface PrioritizedHost {
* The weight of the host to use for load balancing.
* @return the weight of the host to use for load balancing.
*/
double loadBalancingWeight();
double weight();

/**
* Set the weight of the host to use during load balancing.
* @param weight the weight of the host to use during load balancing.
*/
void loadBalancingWeight(double weight);
void weight(double weight);
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ void noPriorities() {
void noPrioritiesWithWeights() {
List<TestPrioritizedHost> hosts = makeHosts(4);
for (int i = 0; i < hosts.size(); i++) {
hosts.get(i).loadBalancingWeight(i + 1d);
hosts.get(i).weight(i + 1d);
}
List<TestPrioritizedHost> result = hostPriorityStrategy.prioritize(hosts);
assertThat(result.size(), equalTo(hosts.size()));
Expand Down Expand Up @@ -79,7 +79,7 @@ void twoPrioritiesNoWeights() {
void twoPrioritiesWithWeights() {
List<TestPrioritizedHost> hosts = makeHosts(6);
for (int i = 0; i < hosts.size(); i++) {
hosts.get(i).loadBalancingWeight(i + 1d);
hosts.get(i).weight(i + 1d);
if (i >= 3) {
hosts.get(i).priority(1);
}
Expand All @@ -99,7 +99,7 @@ void twoPrioritiesWithWeights() {
void twoPrioritiesWithWeightsButSkipNumbers() {
List<TestPrioritizedHost> hosts = makeHosts(6);
for (int i = 0; i < hosts.size(); i++) {
hosts.get(i).loadBalancingWeight(i + 1d);
hosts.get(i).weight(i + 1d);
if (i >= 3) {
hosts.get(i).priority(2);
}
Expand All @@ -119,7 +119,7 @@ void twoPrioritiesWithWeightsButSkipNumbers() {
void noHealthyNodesDoesntFilterOutElements() {
List<TestPrioritizedHost> hosts = makeHosts(6);
for (int i = 0; i < hosts.size(); i++) {
hosts.get(i).loadBalancingWeight(i + 1d);
hosts.get(i).weight(i + 1d);
hosts.get(i).isHealthy = false;
if (i >= 3) {
hosts.get(i).priority(1);
Expand Down Expand Up @@ -206,10 +206,10 @@ void priorityGroupsWithWeightedUnhealthyNodes() {
hosts.get(0).isHealthy(false);
for (int i = 0; i < hosts.size(); i++) {
if (i >= 3) {
hosts.get(i).loadBalancingWeight(i - 3 + 1d);
hosts.get(i).weight(i - 3 + 1d);
hosts.get(i).priority(1);
} else {
hosts.get(i).loadBalancingWeight(i + 1d);
hosts.get(i).weight(i + 1d);
}
}
List<TestPrioritizedHost> result = hostPriorityStrategy.prioritize(hosts);
Expand Down Expand Up @@ -241,10 +241,10 @@ void onlyHealthyNodesAreZeroWeight() {
List<TestPrioritizedHost> hosts = makeHosts(6);
for (int i = 0; i < hosts.size(); i++) {
if (i >= 3) {
hosts.get(i).loadBalancingWeight(0);
hosts.get(i).weight(0);
hosts.get(i).priority(1);
} else {
hosts.get(i).loadBalancingWeight(1);
hosts.get(i).weight(1);
hosts.get(i).isHealthy = false;
}
}
Expand Down Expand Up @@ -312,7 +312,7 @@ void priority(final int priority) {
// Set the weight to use in load balancing. This includes derived weight information such as prioritization
// and is what the host selectors will use when picking endpoints.
@Override
public void loadBalancingWeight(final double weight) {
public void weight(final double weight) {
this.loadBalancedWeight = weight;
}

Expand All @@ -330,7 +330,7 @@ public void isHealthy(final boolean isHealthy) {
}

@Override
public double loadBalancingWeight() {
public double weight() {
return loadBalancedWeight;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,10 @@ void changesToPriorityOrWeightTriggerRebuilds() throws Exception {
public <T extends PrioritizedHost> List<T> prioritize(List<T> hosts) {
assert hosts.size() == 1;
T host = hosts.get(0);
value.set(host.loadBalancingWeight());
value.set(host.weight());
// We want to adjust the weight here so that if the `loadBalancingWeight()` fails to be
// reset then the test will fail.
host.loadBalancingWeight(0.5 * host.loadBalancingWeight());
host.weight(0.5 * host.weight());
return hosts;
}
};
Expand All @@ -201,23 +201,23 @@ public <T extends PrioritizedHost> List<T> prioritize(List<T> hosts) {
List<? extends DefaultLoadBalancer.PrioritizedHostImpl<?, ?>> curentHosts = refinedLb.hosts();
assertThat(curentHosts, hasSize(1));
assertThat(curentHosts.get(0).priority(), equalTo(0));
assertThat(curentHosts.get(0).loadBalancingWeight(), equalTo(0.5));
assertThat(curentHosts.get(0).weight(), equalTo(0.5));
assertThat(value.getAndSet(null), equalTo(1.0));

// send a new event with a different priority group. This should trigger another build.
sendServiceDiscoveryEvents(richEvent(upEvent("address-1"), 1.0, 1));
curentHosts = refinedLb.hosts();
assertThat(curentHosts, hasSize(1));
assertThat(curentHosts.get(0).priority(), equalTo(1));
assertThat(curentHosts.get(0).loadBalancingWeight(), equalTo(0.5));
assertThat(curentHosts.get(0).weight(), equalTo(0.5));
assertThat(value.getAndSet(null), equalTo(1.0));

// send a new event with a different weight. This should trigger yet another build.
sendServiceDiscoveryEvents(richEvent(upEvent("address-1"), 2.0, 1));
curentHosts = refinedLb.hosts();
assertThat(curentHosts, hasSize(1));
assertThat(curentHosts.get(0).priority(), equalTo(1));
assertThat(curentHosts.get(0).loadBalancingWeight(), equalTo(1.0));
assertThat(curentHosts.get(0).weight(), equalTo(1.0));
assertThat(value.getAndSet(null), equalTo(2.0));
}

Expand Down

0 comments on commit 5fb80d0

Please sign in to comment.