Skip to content

Commit

Permalink
Fix javadoc to reflect current behavior of getSubscribedResources. …
Browse files Browse the repository at this point in the history
…Use existing function rather than calculating active authorities and make that function return a set when there are large numbers of authorities.
  • Loading branch information
larry-safran committed Nov 19, 2024
1 parent 1a651c9 commit 08aa9b3
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
5 changes: 3 additions & 2 deletions xds/src/main/java/io/grpc/xds/client/XdsClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -413,8 +413,9 @@ void handleResourceResponse(
public interface ResourceStore {

/**
* Returns the collection of resources currently subscribing to with the specified authority
* or {@code null} if not subscribing to any resources for this authority for the given type.
* Returns the collection of resources currently subscribed to which have an authority matching
* one of those for which the ControlPlaneClient associated with the specified ServerInfo is
* the active one, or {@code null} if no such resources are currently subscribed to.
*
* <p>Note an empty collection indicates subscribing to resources of the given type with
* wildcard mode.
Expand Down
16 changes: 8 additions & 8 deletions xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,7 @@ public Collection<String> getSubscribedResources(
return null;
}

Set<String> authorities = activatedCpClients.entrySet().stream()
.filter(entry -> entry.getValue().contains(targetCpc))
.map(Map.Entry::getKey)
.collect(Collectors.toSet());
Collection<String> authorities = getActiveAuthorities(targetCpc);

Map<String, ResourceSubscriber<? extends ResourceUpdate>> resources =
resourceSubscribers.getOrDefault(type, Collections.emptyMap());
Expand Down Expand Up @@ -432,7 +429,7 @@ private Set<String> getResourceKeys(XdsResourceType<?> xdsResourceType) {

// cpcForThisStream is null when doing shutdown
private void cleanUpResourceTimers(ControlPlaneClient cpcForThisStream) {
List<String> authoritiesForCpc = getActiveAuthorities(cpcForThisStream);
Collection<String> authoritiesForCpc = getActiveAuthorities(cpcForThisStream);

for (Map<String, ResourceSubscriber<?>> subscriberMap : resourceSubscribers.values()) {
for (ResourceSubscriber<?> subscriber : subscriberMap.values()) {
Expand Down Expand Up @@ -964,7 +961,7 @@ public void handleStreamClosed(Status status, boolean shouldTryFallback) {

Status error = status.isOk() ? Status.UNAVAILABLE.withDescription(CLOSED_BY_SERVER) : status;

List<String> authoritiesForClosedCpc = getActiveAuthorities(cpcClosed);
Collection<String> authoritiesForClosedCpc = getActiveAuthorities(cpcClosed);
for (Map<String, ResourceSubscriber<? extends ResourceUpdate>> subscriberMap :
resourceSubscribers.values()) {
for (ResourceSubscriber<? extends ResourceUpdate> subscriber : subscriberMap.values()) {
Expand Down Expand Up @@ -1035,12 +1032,15 @@ private CpcWithFallbackState(ControlPlaneClient cpc, boolean didFallback) {
}
}

private List<String> getActiveAuthorities(ControlPlaneClient cpc) {
return activatedCpClients.entrySet().stream()
private Collection<String> getActiveAuthorities(ControlPlaneClient cpc) {
List<String> asList = activatedCpClients.entrySet().stream()
.filter(entry -> !entry.getValue().isEmpty()
&& cpc == entry.getValue().get(entry.getValue().size() - 1))
.map(Map.Entry::getKey)
.collect(Collectors.toList());

// Since this is usually used for contains, use a set when the list is large
return (asList.size() < 100) ? asList : new HashSet<>(asList);
}

}

0 comments on commit 08aa9b3

Please sign in to comment.