Ethanlm commented on a change in pull request #3346:
URL: https://github.com/apache/storm/pull/3346#discussion_r548010927
##########
File path:
storm-server/src/test/java/org/apache/storm/scheduler/resource/strategies/scheduling/TestBackwardCompatibility.java
##########
@@ -131,7 +131,7 @@ protected Class getDefaultResourceAwareStrategyClass() {
}
/**********************************************************************************
- * Tests for testGenericResourceAwareStrategy
+ * Tests for testGenericResourceAwareStrategyWithoutSettingAckerExecutors
Review comment:
This shouldn't change
##########
File path:
storm-server/src/test/java/org/apache/storm/scheduler/resource/strategies/scheduling/TestGenericResourceAwareStrategy.java
##########
@@ -223,25 +249,166 @@ public void testGenericResourceAwareStrategy() {
scheduler.prepare(conf, new StormMetricsRegistry());
scheduler.schedule(topologies, cluster);
- //We need to have 3 slots on 3 separate hosts. The topology needs 6
GPUs 3500 MB memory and 350% CPU
+ // We need to have 3 slots on 3 separate hosts. The topology needs 6
GPUs 3500 MB memory and 350% CPU
// The bolt-3 instances must be on separate nodes because they each
need 2 GPUs.
// The bolt-2 instances must be on the same node as they each need 1
GPU
// (this assumes that we are packing the components to avoid
fragmentation).
// The bolt-1 and spout instances fill in the rest.
+ // Ordered execs: [[6, 6], [2, 2], [4, 4], [5, 5], [1, 1], [3, 3], [0,
0]]
+ // Ackers: [[8, 8], [7, 7]] (+ [[9, 9], [10, 10]] when
numOfAckersPerWorker=2)
+ HashSet<HashSet<ExecutorDetails>> expectedScheduling = new HashSet<>();
+ if (numOfAckersPerWorker == -1 || numOfAckersPerWorker == 1) {
+ expectedScheduling.add(new HashSet<>(Arrays.asList(
+ new ExecutorDetails(3, 3)))); //bolt-3 - 500 MB, 50% CPU, 2 GPU
+ //Total 500 MB, 50% CPU, 2 - GPU -> this node has 1500 MB, 150%
cpu, 0 GPU left
+ expectedScheduling.add(new HashSet<>(Arrays.asList(
+ new ExecutorDetails(6, 6), //bolt-2 - 500 MB, 50% CPU, 1 GPU
+ new ExecutorDetails(2, 2), //bolt-1 - 500 MB, 50% CPU, 0 GPU
+ new ExecutorDetails(5, 5), //bolt-2 - 500 MB, 50% CPU, 1 GPU
+ new ExecutorDetails(8, 8)))); //acker - 250 MB, 50% CPU, 0 GPU
+ //Total 1750 MB, 200% CPU, 2 GPU -> this node has 250 MB, 0% CPU,
0 GPU left
+ expectedScheduling.add(new HashSet<>(Arrays.asList(
+ new ExecutorDetails(4, 4), //bolt-3 500 MB, 50% cpu, 2 GPU
+ new ExecutorDetails(1, 1), //bolt-1 - 500 MB, 50% CPU, 0 GPU
+ new ExecutorDetails(0, 0), //Spout - 500 MB, 50% CPU, 0 GPU
+ new ExecutorDetails(7, 7) ))); //acker - 250 MB, 50% CPU, 0 GPU
+ //Total 1750 MB, 200% CPU, 2 GPU -> this node has 250 MB, 0% CPU,
0 GPU left
+ } else if (numOfAckersPerWorker == 0) {
+ expectedScheduling.add(new HashSet<>(Arrays.asList(
+ new ExecutorDetails(3, 3)))); //bolt-3 - 500 MB, 50% CPU, 2 GPU
+ //Total 500 MB, 50% CPU, 2 - GPU -> this node has 1500 MB, 150%
cpu, 0 GPU left
+ expectedScheduling.add(new HashSet<>(Arrays.asList(
+ new ExecutorDetails(6, 6), //bolt-2 - 500 MB, 50% CPU, 1 GPU
+ new ExecutorDetails(2, 2), //bolt-1 - 500 MB, 50% CPU, 0 GPU
+ new ExecutorDetails(5, 5), //bolt-2 - 500 MB, 50% CPU, 1 GPU
+ new ExecutorDetails(1, 1)))); //bolt-1 - 500 MB, 50% CPU, 0
GPU
+ //Total 2000 MB, 200% CPU, 2 GPU -> this node has 0 MB, 0% CPU, 0
GPU left
+ expectedScheduling.add(new HashSet<>(Arrays.asList(
+ new ExecutorDetails(0, 0), //Spout - 500 MB, 50% CPU, 0 GPU
+ new ExecutorDetails(4, 4)))); //bolt-3 500 MB, 50% cpu, 2 GPU
+ //Total 1000 MB, 100% CPU, 2 GPU -> this node has 1000 MB, 100%
CPU, 0 GPU left
+ } else if (numOfAckersPerWorker == 2) {
+ expectedScheduling.add(new HashSet<>(Arrays.asList(
+ new ExecutorDetails(3, 3)))); //bolt-3 - 500 MB, 50% CPU, 2 GPU
+ //Total 500 MB, 50% CPU, 2 - GPU -> this node has 1500 MB, 150%
cpu, 0 GPU left
+ expectedScheduling.add(new HashSet<>(Arrays.asList(
+ new ExecutorDetails(7, 7), //acker - 250 MB, 50% CPU, 0
GPU
+ new ExecutorDetails(8, 8), //acker - 250 MB, 50% CPU, 0
GPU
+ new ExecutorDetails(6, 6), //bolt-2 - 500 MB, 50% CPU, 1
GPU
+ new ExecutorDetails(2, 2)))); //bolt-1 - 500 MB, 50% CPU, 0
GPU
+ //Total 1500 MB, 200% CPU, 2 GPU -> this node has 500 MB, 0% CPU,
0 GPU left
+ expectedScheduling.add(new HashSet<>(Arrays.asList(
+ new ExecutorDetails(9, 9), //acker- 250 MB, 50% CPU, 0 GPU
+ new ExecutorDetails(10, 10), //acker- 250 MB, 50% CPU, 0 GPU
+ new ExecutorDetails(1, 1), //bolt-1 - 500 MB, 50% CPU, 0 GPU
+ new ExecutorDetails(4, 4)))); //bolt-3 500 MB, 50% cpu, 2 GPU
+ //Total 1500 MB, 200% CPU, 2 GPU -> this node has 500 MB, 0% CPU,
0 GPU left
+ expectedScheduling.add(new HashSet<>(Arrays.asList(
+ new ExecutorDetails(0, 0), //Spout - 500 MB, 50% CPU, 0 GPU
+ new ExecutorDetails(5, 5)))); //bolt-2 - 500 MB, 50% CPU, 1 GPU
+ //Total 1000 MB, 100% CPU, 2 GPU -> this node has 1000 MB, 100%
CPU, 0 GPU left
+ }
+ HashSet<HashSet<ExecutorDetails>> foundScheduling = new HashSet<>();
+ SchedulerAssignment assignment =
cluster.getAssignmentById("testTopology-id");
+ for (Collection<ExecutorDetails> execs :
assignment.getSlotToExecutors().values()) {
+ foundScheduling.add(new HashSet<>(execs));
+ }
+
+ assertEquals(expectedScheduling, foundScheduling);
+ }
+
+ /**
+ * Test if the scheduling logic for the GenericResourceAwareStrategy is
correct
+ * with setting acker executors.
Review comment:
use `TOPOLOGY_ACKER_EXECUTORS` looks more clear to me
##########
File path:
storm-server/src/test/java/org/apache/storm/scheduler/resource/strategies/scheduling/TestGenericResourceAwareStrategy.java
##########
@@ -181,10 +185,15 @@ public void
testGenericResourceAwareStrategySharedMemory() {
}
/**
- * test if the scheduling logic for the GenericResourceAwareStrategy is
correct.
+ * Test if the scheduling logic for the GenericResourceAwareStrategy is
correct
+ * without setting acker executors.
Review comment:
It would be easier to understand if we replace "acker executors" with
TOPOLOGY_ACKER_EXECUTORS here.
##########
File path:
storm-server/src/test/java/org/apache/storm/scheduler/resource/strategies/scheduling/TestBackwardCompatibility.java
##########
@@ -362,13 +365,9 @@ public void
testDefaultResourceAwareStrategySharedMemory(TestDefaultResourceAwar
}
@Test
- public void testDefaultResourceAwareStrategy() {
- testDefaultResourceAwareStrategy.testDefaultResourceAwareStrategy();
- }
-
- @Test
- public void testDefaultResourceAwareStrategyInFavorOfShuffle() {
-
testDefaultResourceAwareStrategy.testDefaultResourceAwareStrategyInFavorOfShuffle();
Review comment:
should we keep this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
