Skip to content

Commit da995b7

Browse files
committed
xds: Avoid changing cache when watching children in XdsDepManager
The watchers can be completely regular, so the base class can do the cache management while the subclasses are only concerned with subscribing to children.
1 parent 6afacf5 commit da995b7

File tree

1 file changed

+22
-52
lines changed

1 file changed

+22
-52
lines changed

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

Lines changed: 22 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,8 @@ private static void addConfigForCluster(
323323
Status.INTERNAL.withDescription("Logical DNS in dependency manager unsupported")));
324324
break;
325325
default:
326-
throw new IllegalStateException("Unexpected value: " + cdsUpdate.clusterType());
326+
child = new EndpointConfig(StatusOr.fromStatus(Status.UNAVAILABLE.withDescription(
327+
"Unknown type in cluster " + clusterName + " " + cdsUpdate.clusterType())));
327328
}
328329
clusters.put(clusterName, StatusOr.fromValue(
329330
new XdsConfig.XdsClusterConfig(clusterName, cdsUpdate, child)));
@@ -505,7 +506,7 @@ public void onError(Status error) {
505506
}
506507
// Don't update configuration on error, if we've already received configuration
507508
if (!hasDataValue()) {
508-
setDataAsStatus(Status.UNAVAILABLE.withDescription(
509+
this.data = StatusOr.fromStatus(Status.UNAVAILABLE.withDescription(
509510
String.format("Error retrieving %s: %s: %s",
510511
toContextString(), error.getCode(), error.getDescription())));
511512
maybePublishConfig();
@@ -519,11 +520,25 @@ public void onResourceDoesNotExist(String resourceName) {
519520
}
520521

521522
checkArgument(this.resourceName.equals(resourceName), "Resource name does not match");
522-
setDataAsStatus(Status.UNAVAILABLE.withDescription(
523+
this.data = StatusOr.fromStatus(Status.UNAVAILABLE.withDescription(
523524
toContextString() + " does not exist" + nodeInfo()));
524525
maybePublishConfig();
525526
}
526527

528+
@Override
529+
public void onChanged(T update) {
530+
checkNotNull(update, "update");
531+
if (cancelled) {
532+
return;
533+
}
534+
535+
this.data = StatusOr.fromValue(update);
536+
subscribeToChildren(update);
537+
maybePublishConfig();
538+
}
539+
540+
protected abstract void subscribeToChildren(T update);
541+
527542
public void close() {
528543
cancelled = true;
529544
xdsClient.cancelXdsResourceWatch(type, resourceName, this);
@@ -542,20 +557,6 @@ boolean hasDataValue() {
542557
return data != null && data.hasValue();
543558
}
544559

545-
String resourceName() {
546-
return resourceName;
547-
}
548-
549-
protected void setData(T data) {
550-
checkNotNull(data, "data");
551-
this.data = StatusOr.fromValue(data);
552-
}
553-
554-
protected void setDataAsStatus(Status status) {
555-
checkNotNull(status, "status");
556-
this.data = StatusOr.fromStatus(status);
557-
}
558-
559560
public String toContextString() {
560561
return toContextStr(type.typeName(), resourceName);
561562
}
@@ -573,12 +574,7 @@ private LdsWatcher(String resourceName) {
573574
}
574575

575576
@Override
576-
public void onChanged(XdsListenerResource.LdsUpdate update) {
577-
checkNotNull(update, "update");
578-
if (cancelled) {
579-
return;
580-
}
581-
577+
public void subscribeToChildren(XdsListenerResource.LdsUpdate update) {
582578
HttpConnectionManager httpConnectionManager = update.httpConnectionManager();
583579
List<VirtualHost> virtualHosts;
584580
if (httpConnectionManager == null) {
@@ -595,9 +591,6 @@ public void onChanged(XdsListenerResource.LdsUpdate update) {
595591
if (rdsName != null) {
596592
addRdsWatcher(rdsName);
597593
}
598-
599-
setData(update);
600-
maybePublishConfig();
601594
}
602595

603596
private String getRdsName(XdsListenerResource.LdsUpdate update) {
@@ -665,14 +658,8 @@ public RdsWatcher(String resourceName) {
665658
}
666659

667660
@Override
668-
public void onChanged(RdsUpdate update) {
669-
checkNotNull(update, "update");
670-
if (cancelled) {
671-
return;
672-
}
673-
setData(update);
661+
public void subscribeToChildren(RdsUpdate update) {
674662
updateRoutes(update.virtualHosts);
675-
maybePublishConfig();
676663
}
677664

678665
@Override
@@ -690,31 +677,20 @@ private class CdsWatcher extends XdsWatcherBase<XdsClusterResource.CdsUpdate> {
690677
}
691678

692679
@Override
693-
public void onChanged(XdsClusterResource.CdsUpdate update) {
694-
checkNotNull(update, "update");
695-
if (cancelled) {
696-
return;
697-
}
680+
public void subscribeToChildren(XdsClusterResource.CdsUpdate update) {
698681
switch (update.clusterType()) {
699682
case EDS:
700-
setData(update);
701683
addEdsWatcher(getEdsServiceName());
702684
break;
703685
case LOGICAL_DNS:
704-
setData(update);
705686
// no eds needed
706687
break;
707688
case AGGREGATE:
708-
setData(update);
709689
update.prioritizedClusterNames()
710690
.forEach(name -> addClusterWatcher(name));
711691
break;
712692
default:
713-
Status error = Status.UNAVAILABLE.withDescription(
714-
"unknown cluster type in " + resourceName() + " " + update.clusterType());
715-
setDataAsStatus(error);
716693
}
717-
maybePublishConfig();
718694
}
719695

720696
public String getEdsServiceName() {
@@ -734,12 +710,6 @@ private EdsWatcher(String resourceName) {
734710
}
735711

736712
@Override
737-
public void onChanged(XdsEndpointResource.EdsUpdate update) {
738-
if (cancelled) {
739-
return;
740-
}
741-
setData(checkNotNull(update, "update"));
742-
maybePublishConfig();
743-
}
713+
public void subscribeToChildren(XdsEndpointResource.EdsUpdate update) {}
744714
}
745715
}

0 commit comments

Comments
 (0)