xds: CHANNEL_ID hash should be random for the channel

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.
This commit is contained in:
Eric Anderson 2022-08-18 17:27:15 -07:00 committed by GitHub
parent 81abb21e7f
commit 618a4de705
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 4 additions and 1 deletions

View File

@ -123,6 +123,7 @@ final class XdsNameResolver extends NameResolver {
// put()/remove() must be called in SyncContext, and get() can be called in any thread. // 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 ConcurrentMap<String, ClusterRefState> clusterRefs = new ConcurrentHashMap<>();
private final ConfigSelector configSelector = new ConfigSelector(); private final ConfigSelector configSelector = new ConfigSelector();
private final long randomChannelId;
private volatile RoutingConfig routingConfig = RoutingConfig.empty; private volatile RoutingConfig routingConfig = RoutingConfig.empty;
private Listener2 listener; private Listener2 listener;
@ -162,6 +163,7 @@ final class XdsNameResolver extends NameResolver {
this.xdsClientPoolFactory.setBootstrapOverride(bootstrapOverride); this.xdsClientPoolFactory.setBootstrapOverride(bootstrapOverride);
this.random = checkNotNull(random, "random"); this.random = checkNotNull(random, "random");
this.filterRegistry = checkNotNull(filterRegistry, "filterRegistry"); this.filterRegistry = checkNotNull(filterRegistry, "filterRegistry");
randomChannelId = random.nextLong();
logId = InternalLogId.allocate("xds-resolver", name); logId = InternalLogId.allocate("xds-resolver", name);
logger = XdsLogger.withLogId(logId); logger = XdsLogger.withLogId(logId);
logger.log(XdsLogLevel.INFO, "Created resolver for {0}", name); logger.log(XdsLogLevel.INFO, "Created resolver for {0}", name);
@ -586,7 +588,7 @@ final class XdsNameResolver extends NameResolver {
newHash = hashFunc.hashAsciiString(value); newHash = hashFunc.hashAsciiString(value);
} }
} else if (policy.type() == HashPolicy.Type.CHANNEL_ID) { } else if (policy.type() == HashPolicy.Type.CHANNEL_ID) {
newHash = hashFunc.hashLong(logId.getId()); newHash = hashFunc.hashLong(randomChannelId);
} }
if (newHash != null ) { if (newHash != null ) {
// Rotating the old value prevents duplicate hash rules from cancelling each other out // Rotating the old value prevents duplicate hash rules from cancelling each other out

View File

@ -802,6 +802,7 @@ public class XdsNameResolverTest {
// A different resolver/Channel. // A different resolver/Channel.
resolver.shutdown(); resolver.shutdown();
reset(mockListener); reset(mockListener);
when(mockRandom.nextLong()).thenReturn(123L);
resolver = new XdsNameResolver(null, AUTHORITY, null, serviceConfigParser, resolver = new XdsNameResolver(null, AUTHORITY, null, serviceConfigParser,
syncContext, scheduler, syncContext, scheduler,
xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null);