Skip to content

Commit d2c53f6

Browse files
committed
Fix the bug of setting the wrong field.
1 parent eb5f89d commit d2c53f6

File tree

2 files changed

+20
-22
lines changed

2 files changed

+20
-22
lines changed

xds/src/main/java/io/grpc/xds/XdsNameResolver.java

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -355,23 +355,22 @@ public Result selectConfig(PickSubchannelArgs args) {
355355
}
356356
} while (!retainCluster(cluster));
357357
// TODO(chengyuanzhang): avoid service config generation and parsing for each call.
358-
Object config = null;
359358
long timeoutNano = selectedRoute.getRouteAction().getTimeoutNano();
360359
if (timeoutNano == 0) {
361360
timeoutNano = fallbackTimeoutNano;
362361
}
362+
Map<String, ?> rawServiceConfig = Collections.emptyMap();
363363
if (timeoutNano > 0) {
364-
Map<String, ?> rawServiceConfig =
365-
generateServiceConfigWithMethodTimeoutConfig(timeoutNano);
366-
ConfigOrError parsedServiceConfig =
367-
serviceConfigParser.parseServiceConfig(rawServiceConfig);
368-
config = parsedServiceConfig.getConfig();
369-
if (config == null) {
370-
releaseCluster(cluster);
371-
return Result.forError(
372-
parsedServiceConfig.getError().augmentDescription(
373-
"Failed to parse service config (method config)"));
374-
}
364+
rawServiceConfig = generateServiceConfigWithMethodTimeoutConfig(timeoutNano);
365+
}
366+
ConfigOrError parsedServiceConfig =
367+
serviceConfigParser.parseServiceConfig(rawServiceConfig);
368+
Object config = parsedServiceConfig.getConfig();
369+
if (config == null) {
370+
releaseCluster(cluster);
371+
return Result.forError(
372+
parsedServiceConfig.getError().augmentDescription(
373+
"Failed to parse service config (method config)"));
375374
}
376375
final String finalCluster = cluster;
377376
class SelectionCompleted implements Runnable {
@@ -381,14 +380,12 @@ public void run() {
381380
}
382381
}
383382

384-
Result.Builder resultBuilder =
383+
return
385384
Result.newBuilder()
386385
.setCallOptions(args.getCallOptions().withOption(CLUSTER_SELECTION_KEY, cluster))
387-
.setCommittedCallback(new SelectionCompleted());
388-
if (config != null) {
389-
resultBuilder.setConfig(config);
390-
}
391-
return resultBuilder.build();
386+
.setConfig(config)
387+
.setCommittedCallback(new SelectionCompleted())
388+
.build();
392389
}
393390

394391
private boolean retainCluster(String cluster) {
@@ -448,7 +445,7 @@ public void onChanged(LdsUpdate update) {
448445
}
449446
cleanUpRdsWatcher();
450447
if (virtualHosts != null) {
451-
updateRoutes(virtualHosts, httpMaxStreamDurationNano);
448+
updateRoutes(virtualHosts);
452449
} else {
453450
rdsResource = rdsName;
454451
rdsWatcher = new RdsResourceWatcherImpl();
@@ -483,7 +480,7 @@ private void stop() {
483480
xdsClient.cancelLdsResourceWatch(authority, this);
484481
}
485482

486-
private void updateRoutes(List<VirtualHost> virtualHosts, long fallbackTimeoutNano) {
483+
private void updateRoutes(List<VirtualHost> virtualHosts) {
487484
VirtualHost virtualHost = findVirtualHostForHostName(virtualHosts, authority);
488485
if (virtualHost == null) {
489486
logger.log(XdsLogLevel.WARNING,
@@ -552,7 +549,7 @@ private class RdsResourceWatcherImpl implements RdsResourceWatcher {
552549

553550
@Override
554551
public void onChanged(RdsUpdate update) {
555-
updateRoutes(update.getVirtualHosts(), httpMaxStreamDurationNano);
552+
updateRoutes(update.getVirtualHosts());
556553
}
557554

558555
@Override

xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ private List<VirtualHost> buildUnmatchedVirtualHosts() {
290290
Collections.singletonList(route2)));
291291
}
292292

293+
@SuppressWarnings("unchecked")
293294
@Test
294295
public void resolved_noTimeout() {
295296
resolver.start(mockListener);
@@ -307,7 +308,7 @@ public void resolved_noTimeout() {
307308
assertThat(selectResult.getStatus().isOk()).isTrue();
308309
assertThat(selectResult.getCallOptions().getOption(XdsNameResolver.CLUSTER_SELECTION_KEY))
309310
.isEqualTo(cluster1);
310-
assertThat(selectResult.getConfig()).isNull();
311+
assertThat((Map<String, ?>) selectResult.getConfig()).isEmpty();
311312
}
312313

313314
@Test

0 commit comments

Comments
 (0)