Skip to content

Commit efe9ccc

Browse files
authored
xds: Non-SOTW resources need onError() callbacks, too (#12122)
SOTW is unique in that it can become absent after being found. But if we NACK when initially loading the resource, we don't want to delay, depend on the resource timeout, and then give a poor error. This was noticed while adding the EDS restriction that address is not a hostname and some tests started hanging instead of failing quickly.
1 parent 48d08e6 commit efe9ccc

File tree

2 files changed

+7
-6
lines changed

2 files changed

+7
-6
lines changed

xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -592,12 +592,6 @@ private <T extends ResourceUpdate> void handleResourceUpdate(
592592
subscriber.onRejected(args.versionInfo, updateTime, errorDetail);
593593
}
594594

595-
// Nothing else to do for incremental ADS resources.
596-
if (!xdsResourceType.isFullStateOfTheWorld()) {
597-
continue;
598-
}
599-
600-
// Handle State of the World ADS: invalid resources.
601595
if (invalidResources.contains(resourceName)) {
602596
// The resource is missing. Reuse the cached resource if possible.
603597
if (subscriber.data == null) {
@@ -607,6 +601,11 @@ private <T extends ResourceUpdate> void handleResourceUpdate(
607601
continue;
608602
}
609603

604+
// Nothing else to do for incremental ADS resources.
605+
if (!xdsResourceType.isFullStateOfTheWorld()) {
606+
continue;
607+
}
608+
610609
// For State of the World services, notify watchers when their watched resource is missing
611610
// from the ADS update. Note that we can only do this if the resource update is coming from
612611
// the same xDS server that the ResourceSubscriber is subscribed to.

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3270,6 +3270,8 @@ public void edsDuplicateLocalityInTheSamePriority() {
32703270
+ "locality:Locality{region=region2, zone=zone2, subZone=subzone2} for priority:1";
32713271
call.verifyRequestNack(EDS, EDS_RESOURCE, "", "0001", NODE, ImmutableList.of(
32723272
errorMsg));
3273+
verify(edsResourceWatcher).onError(errorCaptor.capture());
3274+
assertThat(errorCaptor.getValue().getDescription()).contains(errorMsg);
32733275
}
32743276

32753277
@Test

0 commit comments

Comments
 (0)