Updated the config reading logic.

Project: http://git-wip-us.apache.org/repos/asf/twill/repo
Commit: http://git-wip-us.apache.org/repos/asf/twill/commit/f9d5a596
Tree: http://git-wip-us.apache.org/repos/asf/twill/tree/f9d5a596
Diff: http://git-wip-us.apache.org/repos/asf/twill/diff/f9d5a596

Branch: refs/heads/feature/TWILL-241-per-runnable-opts
Commit: f9d5a596b0d99c0ca1830113d25d30f520f85bc9
Parents: 29a7999
Author: Terence Yim <[email protected]>
Authored: Fri Aug 4 19:00:00 2017 -0700
Committer: Terence Yim <[email protected]>
Committed: Fri Aug 4 19:00:00 2017 -0700

----------------------------------------------------------------------
 .../internal/TwillRuntimeSpecification.java     | 41 +++++++++++++-------
 .../apache/twill/yarn/ContainerSizeTestRun.java | 14 +++++--
 2 files changed, 39 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/twill/blob/f9d5a596/twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
----------------------------------------------------------------------
diff --git 
a/twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
 
b/twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
index 636d94d..2974870 100644
--- 
a/twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
+++ 
b/twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
@@ -90,33 +90,30 @@ public class TwillRuntimeSpecification {
    * Returns the minimum heap ratio for the application master.
    */
   public double getAMMinHeapRatio() {
-    return getMinHeapRatio(config);
+    return getMinHeapRatio(config, Configs.Defaults.HEAP_RESERVED_MIN_RATIO);
   }
 
   /**
    * Returns the minimum heap ratio for the given runnable.
    */
   public double getMinHeapRatio(String runnableName) {
-    return getMinHeapRatio(runnableConfigs.containsKey(runnableName) ? 
runnableConfigs.get(runnableName) : config);
+    double ratio = getMinHeapRatio(runnableConfigs.get(runnableName), 0d);
+    return ratio <= 0d ? getMinHeapRatio(config, 
Configs.Defaults.HEAP_RESERVED_MIN_RATIO) : ratio;
   }
 
   /**
    * Returns the reserved non-heap memory size in MB for the application 
master.
    */
   public int getAMReservedMemory() {
-    return config.containsKey(Configs.Keys.YARN_AM_RESERVED_MEMORY_MB) ?
-      Integer.parseInt(config.get(Configs.Keys.YARN_AM_RESERVED_MEMORY_MB)) :
-      Configs.Defaults.YARN_AM_RESERVED_MEMORY_MB;
+    return getReservedMemory(config, 
Configs.Defaults.YARN_AM_RESERVED_MEMORY_MB);
   }
 
   /**
    * Returns the reserved non-heap memory size in MB for the given runnable.
    */
   public int getReservedMemory(String runnableName) {
-    Map<String, String> conf = runnableConfigs.containsKey(runnableName) ? 
runnableConfigs.get(runnableName) : config;
-    return conf.containsKey(Configs.Keys.JAVA_RESERVED_MEMORY_MB) ?
-      Integer.parseInt(conf.get(Configs.Keys.JAVA_RESERVED_MEMORY_MB)) :
-      Configs.Defaults.JAVA_RESERVED_MEMORY_MB;
+    int memory = getReservedMemory(runnableConfigs.get(runnableName), -1);
+    return memory < 0 ? getReservedMemory(config, 
Configs.Defaults.JAVA_RESERVED_MEMORY_MB) : memory;
   }
 
   /**
@@ -171,9 +168,27 @@ public class TwillRuntimeSpecification {
   /**
    * Returns the minimum heap ratio ({@link 
Configs.Keys#HEAP_RESERVED_MIN_RATIO}) based on the given configuration.
    */
-  private double getMinHeapRatio(Map<String, String> config) {
-    return config.containsKey(Configs.Keys.HEAP_RESERVED_MIN_RATIO) ?
-      Double.parseDouble(config.get(Configs.Keys.HEAP_RESERVED_MIN_RATIO)) :
-      Configs.Defaults.HEAP_RESERVED_MIN_RATIO;
+  private double getMinHeapRatio(@Nullable Map<String, String> config, double 
defaultValue) {
+    if (config == null) {
+      return defaultValue;
+    }
+
+    double ratio = config.containsKey(Configs.Keys.HEAP_RESERVED_MIN_RATIO) ?
+      Double.parseDouble(config.get(Configs.Keys.HEAP_RESERVED_MIN_RATIO)) : 
defaultValue;
+    // Ratio can't be <= 0
+    return ratio <= 0d ? defaultValue : ratio;
+  }
+
+  /**
+   * Returns the reserved memory size ({@link 
Configs.Keys#HEAP_RESERVED_MIN_RATIO}) based on the given configuration.
+   */
+  private int getReservedMemory(@Nullable Map<String, String> config, int 
defaultValue) {
+    if (config == null) {
+      return defaultValue;
+    }
+    int memory = config.containsKey(Configs.Keys.JAVA_RESERVED_MEMORY_MB) ?
+      Integer.parseInt(config.get(Configs.Keys.JAVA_RESERVED_MEMORY_MB)) : 
defaultValue;
+    // memory size can't be -ve
+    return memory < 0 ? defaultValue : memory;
   }
 }

http://git-wip-us.apache.org/repos/asf/twill/blob/f9d5a596/twill-yarn/src/test/java/org/apache/twill/yarn/ContainerSizeTestRun.java
----------------------------------------------------------------------
diff --git 
a/twill-yarn/src/test/java/org/apache/twill/yarn/ContainerSizeTestRun.java 
b/twill-yarn/src/test/java/org/apache/twill/yarn/ContainerSizeTestRun.java
index 73f1476..10dcaf5 100644
--- a/twill-yarn/src/test/java/org/apache/twill/yarn/ContainerSizeTestRun.java
+++ b/twill-yarn/src/test/java/org/apache/twill/yarn/ContainerSizeTestRun.java
@@ -69,14 +69,13 @@ public class ContainerSizeTestRun extends BaseYarnTest {
   @Test
   public void testMaxHeapSize() throws InterruptedException, TimeoutException, 
ExecutionException {
     TwillRunner runner = getTwillRunner();
-    String runnableName = "sleep";
 
     TwillController controller = runner.prepare(new MaxHeapApp())
       // Alter the AM container size and heap ratio
       .withConfiguration(ImmutableMap.of(Configs.Keys.YARN_AM_MEMORY_MB, "256",
                                          Configs.Keys.HEAP_RESERVED_MIN_RATIO, 
"0.65"))
       // Use a different heap ratio and reserved memory size for the runnable
-      .withConfiguration(runnableName,
+      .withConfiguration("sleep",
                          ImmutableMap.of(Configs.Keys.HEAP_RESERVED_MIN_RATIO, 
"0.8",
                                          Configs.Keys.JAVA_RESERVED_MEMORY_MB, 
"1024"))
       .addLogHandler(new PrinterLogHandler(new PrintWriter(System.out, true)))
@@ -94,12 +93,20 @@ public class ContainerSizeTestRun extends BaseYarnTest {
                           
resourceReport.getAppMasterResources().getMaxHeapMemoryMB());
 
       // Verify the runnable container heap size
-      Collection<TwillRunResources> runnableResources = 
resourceReport.getRunnableResources(runnableName);
+      Collection<TwillRunResources> runnableResources = 
resourceReport.getRunnableResources("sleep");
       Assert.assertFalse(runnableResources.isEmpty());
       TwillRunResources resources = runnableResources.iterator().next();
       
Assert.assertEquals(Resources.computeMaxHeapSize(resources.getMemoryMB(), 1024, 
0.8d),
                           resources.getMaxHeapMemoryMB());
 
+      // For the sleep2 runnable, we don't set any ratio and reserved memory.
+      // The ratio should get default to 0.65 (app) and reserved memory to 200
+      runnableResources = resourceReport.getRunnableResources("sleep2");
+      Assert.assertFalse(runnableResources.isEmpty());
+      resources = runnableResources.iterator().next();
+      Assert.assertEquals(
+        Resources.computeMaxHeapSize(resources.getMemoryMB(), 
Configs.Defaults.YARN_AM_RESERVED_MEMORY_MB, 0.65d),
+        resources.getMaxHeapMemoryMB());
     } finally {
       controller.terminate().get(120, TimeUnit.SECONDS);
     }
@@ -181,6 +188,7 @@ public class ContainerSizeTestRun extends BaseYarnTest {
         .setName("MaxHeapApp")
         .withRunnable()
         .add("sleep", new MaxHeapRunnable(12345), res).noLocalFiles()
+        .add("sleep2", new MaxHeapRunnable(23456), res).noLocalFiles()
         .anyOrder()
         .build();
     }

Reply via email to