Skip to content

Commit

Permalink
xds: CHANNEL_ID hash should be random for the channel
Browse files Browse the repository at this point in the history
The log id is an incrementing value starting from 0. That means the same
binary on two different machines will have the same hashes for each
consecutive Channel. That was not at all the intension of CHANNEL_ID.
From gRFC A42:

> This can be used in similar situations to where Envoy uses
> connection_properties to hash on the source IP address.
  • Loading branch information
ejona86 committed Aug 19, 2022
1 parent 81abb21 commit 618a4de
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 1 deletion.
4 changes: 3 additions & 1 deletion xds/src/main/java/io/grpc/xds/XdsNameResolver.java
Expand Up @@ -123,6 +123,7 @@ final class XdsNameResolver extends NameResolver {
// put()/remove() must be called in SyncContext, and get() can be called in any thread.
private final ConcurrentMap<String, ClusterRefState> clusterRefs = new ConcurrentHashMap<>();
private final ConfigSelector configSelector = new ConfigSelector();
private final long randomChannelId;

private volatile RoutingConfig routingConfig = RoutingConfig.empty;
private Listener2 listener;
Expand Down Expand Up @@ -162,6 +163,7 @@ final class XdsNameResolver extends NameResolver {
this.xdsClientPoolFactory.setBootstrapOverride(bootstrapOverride);
this.random = checkNotNull(random, "random");
this.filterRegistry = checkNotNull(filterRegistry, "filterRegistry");
randomChannelId = random.nextLong();
logId = InternalLogId.allocate("xds-resolver", name);
logger = XdsLogger.withLogId(logId);
logger.log(XdsLogLevel.INFO, "Created resolver for {0}", name);
Expand Down Expand Up @@ -586,7 +588,7 @@ private long generateHash(List<HashPolicy> hashPolicies, Metadata headers) {
newHash = hashFunc.hashAsciiString(value);
}
} else if (policy.type() == HashPolicy.Type.CHANNEL_ID) {
newHash = hashFunc.hashLong(logId.getId());
newHash = hashFunc.hashLong(randomChannelId);
}
if (newHash != null ) {
// Rotating the old value prevents duplicate hash rules from cancelling each other out
Expand Down
1 change: 1 addition & 0 deletions xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
Expand Up @@ -802,6 +802,7 @@ public void resolved_rpcHashingByChannelId() {
// A different resolver/Channel.
resolver.shutdown();
reset(mockListener);
when(mockRandom.nextLong()).thenReturn(123L);
resolver = new XdsNameResolver(null, AUTHORITY, null, serviceConfigParser,
syncContext, scheduler,
xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null);
Expand Down

0 comments on commit 618a4de

Please sign in to comment.