Skip to content

Commit

Permalink
loadbalancer: cleanup HostSelector List variance (#2795)
Browse files Browse the repository at this point in the history
Motivation:

The HostSelector currently operates on a List<Host> which
doesn't allow us to pass in lists of refined types. This
means we need to make copies of lists just to pass them through
if implementations want to store the refined types.

Modifications:

- Make the HostSelector operate on a List<? extends Host>.
  This continues to give implementations the ability to read
  the list entries, which is all it should do, while allowing
  users of the HostSelector to pass in a list of a refined type.
  • Loading branch information
bryce-anderson authored Jan 8, 2024
1 parent 6b2b65e commit cb8405d
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ abstract class BaseHostSelector<ResolvedAddress, C extends LoadBalancedConnectio
implements HostSelector<ResolvedAddress, C> {

private final String targetResource;
private final List<Host<ResolvedAddress, C>> hosts;
BaseHostSelector(final List<Host<ResolvedAddress, C>> hosts, final String targetResource) {
private final List<? extends Host<ResolvedAddress, C>> hosts;
BaseHostSelector(final List<? extends Host<ResolvedAddress, C>> hosts, final String targetResource) {
this.hosts = hosts;
this.targetResource = requireNonNull(targetResource, "targetResource");
}
Expand Down Expand Up @@ -62,7 +62,7 @@ protected final String getTargetResource() {
return targetResource;
}

protected final Single<C> noActiveHostsFailure(List<Host<ResolvedAddress, C>> usedHosts) {
protected final Single<C> noActiveHostsFailure(List<? extends Host<ResolvedAddress, C>> usedHosts) {
return failed(Exceptions.StacklessNoActiveHostException.newInstance("Failed to pick an active host for " +
getTargetResource() + ". Either all are busy, expired, or unhealthy: " + usedHosts,
this.getClass(), "selectConnection(...)"));
Expand Down Expand Up @@ -92,7 +92,7 @@ private Single<C> noHostsFailure() {
}

private static <ResolvedAddress, C extends LoadBalancedConnection> boolean anyHealthy(
final List<Host<ResolvedAddress, C>> usedHosts) {
final List<? extends Host<ResolvedAddress, C>> usedHosts) {
for (Host<ResolvedAddress, C> host : usedHosts) {
if (host.isHealthy()) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ public Single<C> selectConnection(Predicate<C> selector, @Nullable ContextMap co
}

@Override
public HostSelector<ResolvedAddress, C> rebuildWithHosts(List<Host<ResolvedAddress, C>> hosts) {
public HostSelector<ResolvedAddress, C> rebuildWithHosts(List<? extends Host<ResolvedAddress, C>> hosts) {
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ Single<C> selectConnection(Predicate<C> selector, @Nullable ContextMap context,
* @param hosts the new list of {@link Host}s the returned selector should choose from.
* @return the next selector that should be used for host selection.
*/
HostSelector<ResolvedAddress, C> rebuildWithHosts(List<Host<ResolvedAddress, C>> hosts);
HostSelector<ResolvedAddress, C> rebuildWithHosts(List<? extends Host<ResolvedAddress, C>> hosts);

/**
* Whether the load balancer believes itself to be healthy enough for serving traffic.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@
final class P2CSelector<ResolvedAddress, C extends LoadBalancedConnection>
extends BaseHostSelector<ResolvedAddress, C> {

private final List<Host<ResolvedAddress, C>> hosts;
private final List<? extends Host<ResolvedAddress, C>> hosts;
@Nullable
private final Random random;
private final int maxEffort;
private final boolean failOpen;

P2CSelector(List<Host<ResolvedAddress, C>> hosts, final String targetResource, final int maxEffort,
P2CSelector(List<? extends Host<ResolvedAddress, C>> hosts, final String targetResource, final int maxEffort,
final boolean failOpen, @Nullable final Random random) {
super(hosts, targetResource);
this.hosts = hosts;
Expand All @@ -52,7 +52,7 @@ final class P2CSelector<ResolvedAddress, C extends LoadBalancedConnection>
}

@Override
public HostSelector<ResolvedAddress, C> rebuildWithHosts(List<Host<ResolvedAddress, C>> hosts) {
public HostSelector<ResolvedAddress, C> rebuildWithHosts(List<? extends Host<ResolvedAddress, C>> hosts) {
return new P2CSelector<>(hosts, getTargetResource(), maxEffort, failOpen, random);
}

Expand Down Expand Up @@ -83,8 +83,9 @@ protected Single<C> selectConnection0(Predicate<C> selector, @Nullable ContextMa
}
}

private Single<C> p2c(int size, List<Host<ResolvedAddress, C>> hosts, Random random, Predicate<C> selector,
boolean forceNewConnectionAndReserve, @Nullable ContextMap contextMap) {
private Single<C> p2c(int size, List<? extends Host<ResolvedAddress, C>> hosts, Random random,
Predicate<C> selector, boolean forceNewConnectionAndReserve,
@Nullable ContextMap contextMap) {
// If there are only two hosts we only try once since there is no chance we'll select different hosts
// on further iterations.
Host<ResolvedAddress, C> failOpenHost = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ final class RoundRobinSelector<ResolvedAddress, C extends LoadBalancedConnection
extends BaseHostSelector<ResolvedAddress, C> {

private final AtomicInteger index;
private final List<Host<ResolvedAddress, C>> usedHosts;
private final List<? extends Host<ResolvedAddress, C>> usedHosts;
private final boolean failOpen;

RoundRobinSelector(final List<Host<ResolvedAddress, C>> usedHosts, final String targetResource,
RoundRobinSelector(final List<? extends Host<ResolvedAddress, C>> usedHosts, final String targetResource,
final boolean failOpen) {
this(new AtomicInteger(), usedHosts, targetResource, failOpen);
}

private RoundRobinSelector(final AtomicInteger index, final List<Host<ResolvedAddress, C>> usedHosts,
private RoundRobinSelector(final AtomicInteger index, final List<? extends Host<ResolvedAddress, C>> usedHosts,
final String targetResource, final boolean failOpen) {
super(usedHosts, targetResource);
this.index = index;
Expand Down Expand Up @@ -79,7 +79,7 @@ protected Single<C> selectConnection0(
}

@Override
public HostSelector<ResolvedAddress, C> rebuildWithHosts(@Nonnull List<Host<ResolvedAddress, C>> hosts) {
public HostSelector<ResolvedAddress, C> rebuildWithHosts(@Nonnull List<? extends Host<ResolvedAddress, C>> hosts) {
return new RoundRobinSelector<>(index, hosts, getTargetResource(), failOpen);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,9 @@ public HostSelector<String, TestLoadBalancedConnection> buildSelector(

private class TestSelector implements HostSelector<String, TestLoadBalancedConnection> {

private final List<Host<String, TestLoadBalancedConnection>> hosts;
private final List<? extends Host<String, TestLoadBalancedConnection>> hosts;

TestSelector(final List<Host<String, TestLoadBalancedConnection>> hosts) {
TestSelector(final List<? extends Host<String, TestLoadBalancedConnection>> hosts) {
this.hosts = hosts;
}

Expand All @@ -143,7 +143,7 @@ public Single<TestLoadBalancedConnection> selectConnection(

@Override
public HostSelector<String, TestLoadBalancedConnection> rebuildWithHosts(
List<Host<String, TestLoadBalancedConnection>> hosts) {
List<? extends Host<String, TestLoadBalancedConnection>> hosts) {
rebuilds++;
return new TestSelector(hosts);
}
Expand Down

0 comments on commit cb8405d

Please sign in to comment.