Github user agresch commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2631#discussion_r180767148
  
    --- Diff: 
storm-server/src/main/java/org/apache/storm/scheduler/Cluster.java ---
    @@ -301,17 +304,7 @@ public boolean needsSchedulingRas(TopologyDetails 
topology) {
     
         @Override
         public Set<Integer> getUsedPorts(SupervisorDetails supervisor) {
    -        Set<Integer> usedPorts = new HashSet<>();
    -
    -        for (SchedulerAssignment assignment : assignments.values()) {
    -            for (WorkerSlot slot : 
assignment.getExecutorToSlot().values()) {
    -                if (slot.getNodeId().equals(supervisor.getId())) {
    -                    usedPorts.add(slot.getPort());
    -                }
    -            }
    -        }
    -
    -        return usedPorts;
    +        return nodeToUsedPortsCache.computeIfAbsent(supervisor.getId(), 
(x) -> new 
HashSet<>()).stream().map(WorkerSlot::getPort).collect(Collectors.toSet());
    --- End diff --
    
    It looks like updateScheduledResourcesCache() - called from assign() - 
actually adds the worker slots to nodeToUsedPortsCache.  This may be fine, but 
might be nice to add a Java doc note about this since this is a public method.  



---

Reply via email to