Skip to content

Commit

Permalink
Comments
Browse files Browse the repository at this point in the history
  • Loading branch information
pkoenig10 committed Mar 11, 2024
1 parent 18bd034 commit 5878c99
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.palantir.dialogue.clients;

import com.google.common.annotations.Beta;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.annotations.CheckReturnValue;
import com.palantir.conjure.java.api.config.service.ServicesConfigBlock;
import com.palantir.conjure.java.client.config.ClientConfiguration;
Expand Down Expand Up @@ -159,16 +160,20 @@ public interface PerHostClientFactory {
* Returns a list of channels where each channel will route requests to a single, unique host, even if that host
* returns some 429s.
*/
Refreshable<List<Channel>> getPerHostChannels();
default Refreshable<List<Channel>> getPerHostChannels() {
return getNamedPerHostChannels().map(channels -> ImmutableList.copyOf(channels.values()));
}

default <T> Refreshable<List<T>> getPerHost(Class<T> clientInterface) {
return getNamedPerHost(clientInterface).map(channels -> ImmutableList.copyOf(channels.values()));
}

/**
* Returns a list of channels where each channel will route requests to a single, unique host, even if that host
* returns some 429s. The channels are uniquely identified by a stable, opaque key.
*/
Refreshable<Map<PerHostTarget, Channel>> getNamedPerHostChannels();

<T> Refreshable<List<T>> getPerHost(Class<T> clientInterface);

<T> Refreshable<Map<PerHostTarget, T>> getNamedPerHost(Class<T> clientInterface);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,18 @@
public final class PerHostTarget {

private final TargetUri targetUri;
private final boolean isSelf;

PerHostTarget(TargetUri targetUri, boolean isSelf) {
PerHostTarget(TargetUri targetUri) {
this.targetUri = targetUri;
this.isSelf = isSelf;
}

public TargetUri targetUri() {
return targetUri;
}

public boolean isSelf() {
return isSelf;
}

@Override
public String toString() {
return "Target{targetUri='" + targetUri + "', isSelf=" + isSelf + '}';
return "Target{targetUri='" + targetUri + '}';
}

@Override
Expand All @@ -50,13 +44,11 @@ public boolean equals(Object other) {
return false;
}
PerHostTarget perHostTarget = (PerHostTarget) other;
return targetUri.equals(perHostTarget.targetUri) && isSelf == perHostTarget.isSelf;
return targetUri.equals(perHostTarget.targetUri);
}

@Override
public int hashCode() {
int result = targetUri.hashCode();
result = 31 * result + Boolean.hashCode(isSelf);
return result;
return targetUri.hashCode();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,8 @@
import com.palantir.refreshable.Refreshable;
import com.palantir.tritium.metrics.registry.TaggedMetricRegistry;
import java.net.InetAddress;
import java.net.NetworkInterface;
import java.net.Proxy;
import java.net.ProxySelector;
import java.net.SocketException;
import java.net.URI;
import java.net.URISyntaxException;
import java.security.Provider;
Expand Down Expand Up @@ -246,32 +244,18 @@ public PerHostClientFactory perHost(String serviceName) {
return ImmutableMap.of();
}

Set<InetAddress> selfAddresses;
try {
selfAddresses = NetworkInterface.networkInterfaces()
.flatMap(NetworkInterface::inetAddresses)
.collect(Collectors.toSet());
} catch (SocketException e) {
log.warn("Failed to obtain local addresses from network interfaces", e);
selfAddresses = Set.of();
}

ImmutableMap.Builder<PerHostTarget, Channel> map = ImmutableMap.builder();
for (int i = 0; i < targetUris.size(); i++) {
TargetUri targetUri = targetUris.get(i);
ServiceConfiguration singleUriServiceConf = ServiceConfiguration.builder()
.from(serviceConfiguration)
.uris(ImmutableList.of(targetUri.uri()))
.build();
boolean isSelf = targetUri
.resolvedAddress()
.map(selfAddresses::contains)
.orElse(false);

// subtle gotcha here is that every single one of these has the same channelName,
// which means metrics like the QueuedChannel counter will end up being the sum of all of them.
map.put(
new PerHostTarget(targetUri, isSelf),
new PerHostTarget(targetUri),
cache.getNonReloadingChannel(
params,
singleUriServiceConf,
Expand All @@ -283,23 +267,11 @@ public PerHostClientFactory perHost(String serviceName) {
});

return new PerHostClientFactory() {
@Override
public Refreshable<List<Channel>> getPerHostChannels() {
return perHostChannels.map(channels -> ImmutableList.copyOf(channels.values()));
}

@Override
public Refreshable<Map<PerHostTarget, Channel>> getNamedPerHostChannels() {
return perHostChannels;
}

@Override
public <T> Refreshable<List<T>> getPerHost(Class<T> clientInterface) {
return perHostChannels.map(channels -> channels.values().stream()
.map(channel -> Reflection.callStaticFactoryMethod(clientInterface, channel, params.runtime()))
.collect(ImmutableList.toImmutableList()));
}

@Override
public <T> Refreshable<Map<PerHostTarget, T>> getNamedPerHost(Class<T> clientInterface) {
return perHostChannels.map(channels -> channels.entrySet().stream()
Expand Down Expand Up @@ -569,9 +541,11 @@ private ImmutableList<TargetUri> getTargetUris(
log.warn(
"Failed to parse all URIs, falling back to legacy DNS approach for service '{}'",
SafeArg.of("service", serviceNameForLogging));
return uris.stream().map(TargetUri::of).collect(ImmutableList.toImmutableList());
for (String uri : uris) {
targetUris.add(TargetUri.of(uri));
}
}
return ImmutableList.copyOf(targetUris);
return ImmutableSet.copyOf(targetUris).asList();
}

private static ProxySelector proxySelector(Optional<ProxyConfiguration> proxyConfiguration) {
Expand Down

0 comments on commit 5878c99

Please sign in to comment.