[GitHub] storm pull request #2631: [STORM-3025] Optimize Cluster methods with Caching...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/storm/pull/2631


---

[GitHub] storm pull request #2631: [STORM-3025] Optimize Cluster methods with Caching...

[kishorvpatil]

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

https://github.com/apache/storm/pull/2631#discussion_r180811674
  
--- Diff: 
storm-server/src/main/java/org/apache/storm/scheduler/resource/normalization/NormalizedResourceRequest.java
 ---
@@ -187,4 +187,29 @@ public double getTotalCpu() {
 public NormalizedResources getNormalizedResources() {
 return this.normalizedResources;
 }
+
+public void removeOffHeap(final double offHeap) {
+this.offHeap += offHeap;
+}
+
+public void remove(WorkerResources value) {
--- End diff --

Not used



---

[GitHub] storm pull request #2631: [STORM-3025] Optimize Cluster methods with Caching...

[kishorvpatil]

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

https://github.com/apache/storm/pull/2631#discussion_r180800090
  
--- Diff: 
storm-server/src/main/java/org/apache/storm/scheduler/resource/normalization/NormalizedResourceRequest.java
 ---
@@ -187,4 +187,29 @@ public double getTotalCpu() {
 public NormalizedResources getNormalizedResources() {
 return this.normalizedResources;
 }
+
+public void removeOffHeap(final double offHeap) {
--- End diff --

This is unused method. I was trying some thing else. Let me delete this 
method.


---

[GitHub] storm pull request #2631: [STORM-3025] Optimize Cluster methods with Caching...

[agresch]

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

https://github.com/apache/storm/pull/2631#discussion_r180755909
  
--- Diff: 
storm-server/src/main/java/org/apache/storm/scheduler/Cluster.java ---
@@ -79,8 +81,8 @@
 private Set blackListedHosts = new HashSet<>();
 private INimbus inimbus;
 private final Topologies topologies;
-private final Map scheduledCpuCache = new HashMap<>();
-private final Map scheduledMemoryCache = new 
HashMap<>();
+private final Map> 
nodeToScheduledResourcesCache;
+private final HashMap nodeToUsedPortsCache;
--- End diff --

use Map instead of HashMap?


---

[GitHub] storm pull request #2631: [STORM-3025] Optimize Cluster methods with Caching...

[agresch]

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

https://github.com/apache/storm/pull/2631#discussion_r180792829
  
--- Diff: 
storm-server/src/main/java/org/apache/storm/scheduler/resource/normalization/NormalizedResourceRequest.java
 ---
@@ -187,4 +187,29 @@ public double getTotalCpu() {
 public NormalizedResources getNormalizedResources() {
 return this.normalizedResources;
 }
+
+public void removeOffHeap(final double offHeap) {
--- End diff --

where is this used?  Why increment?


---

[GitHub] storm pull request #2631: [STORM-3025] Optimize Cluster methods with Caching...

[agresch]

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 getUsedPorts(SupervisorDetails supervisor) {
-Set 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.  



---

[GitHub] storm pull request #2631: [STORM-3025] Optimize Cluster methods with Caching...

[agresch]

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

https://github.com/apache/storm/pull/2631#discussion_r180794169
  
--- Diff: 
storm-server/src/main/java/org/apache/storm/scheduler/resource/normalization/NormalizedResourceRequest.java
 ---
@@ -187,4 +187,29 @@ public double getTotalCpu() {
 public NormalizedResources getNormalizedResources() {
 return this.normalizedResources;
 }
+
+public void removeOffHeap(final double offHeap) {
+this.offHeap += offHeap;
+}
+
+public void remove(WorkerResources value) {
--- End diff --

were these used?


---

[GitHub] storm pull request #2631: [STORM-3025] Optimize Cluster methods with Caching...

[kishorvpatil]

GitHub user kishorvpatil opened a pull request:

https://github.com/apache/storm/pull/2631

[STORM-3025] Optimize Cluster methods with Caching to avoid loopy loops 

Optimizing the `Cluster` methods that are called from with high frequency 
calls to speed-up scheduling time on new topologies on large clusters.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/kishorvpatil/incubator-storm storm3025

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/2631.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2631


commit 46486d87fc8a26262d75abc05c1398afceee687c
Author: Kishor Patil 
Date:   2018-04-10T21:42:03Z

Remove loopy loops in scheduler cluster state

commit caf7f885d6c10eaed440b02ec345e863af700762
Author: Kishor Patil 
Date:   2018-04-10T22:10:10Z

Clean up caching vars from Cluster

commit b57654c8a90705029ba3555ccd75d7056051c31c
Author: Kishor Patil 
Date:   2018-04-10T22:42:54Z

Cache supervisor to Used Ports in Cluster state




---