Skip to content

Commit dbb779a

Browse files
ejona86AgraVator
authored andcommitted
xds: Non-SOTW resources need onError() callbacks, too (grpc#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 5f441a0 commit dbb779a

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)