Fix calculation of available job capacity returned in HealthIndicator (…

…#504)
Netflix · May 12, 2017 · 89a14e8 · 89a14e8
1 parent e3814c7
commit 89a14e8
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 3 deletions.
diff --git a/genie-web/src/main/java/com/netflix/genie/web/health/GenieMemoryHealthIndicator.java b/genie-web/src/main/java/com/netflix/genie/web/health/GenieMemoryHealthIndicator.java
@@ -84,8 +84,12 @@ public Health health() {
             .withDetail(NUMBER_RUNNING_JOBS_KEY, this.jobMetricsService.getNumActiveJobs())
             .withDetail(USED_MEMORY_KEY, usedMemory)
             .withDetail(AVAILABLE_MEMORY, availableMemory)
-            .withDetail(AVAILABLE_DEFAULT_JOB_CAPACITY, availableMemory % defaultJobMemory)
-            .withDetail(AVAILABLE_MAX_JOB_CAPACITY, availableMemory % maxJobMemory)
+            .withDetail(
+                AVAILABLE_DEFAULT_JOB_CAPACITY,
+                (availableMemory >= 0 && defaultJobMemory > 0) ? (availableMemory / defaultJobMemory) : 0)
+            .withDetail(
+                AVAILABLE_MAX_JOB_CAPACITY,
+                (availableMemory >= 0 && maxJobMemory > 0) ? (availableMemory / maxJobMemory) : 0)
             .build();
     }
 }
diff --git a/...e-web/src/test/java/com/netflix/genie/web/health/GenieMemoryHealthIndicatorUnitTests.java b/...e-web/src/test/java/com/netflix/genie/web/health/GenieMemoryHealthIndicatorUnitTests.java
@@ -28,6 +28,8 @@
 import org.mockito.Mockito;
 import org.springframework.boot.actuate.health.Status;
 
+import java.util.Map;
+
 /**
  * Unit tests for GenieHealthIndicator.
  *
@@ -43,14 +45,15 @@ public class GenieMemoryHealthIndicatorUnitTests {
 
     private GenieMemoryHealthIndicator genieMemoryHealthIndicator;
     private JobMetricsService jobMetricsService;
+    private JobsProperties jobsProperties;
 
     /**
      * Setup for the tests.
      */
     @Before
     public void setup() {
         this.jobMetricsService = Mockito.mock(JobMetricsService.class);
-        final JobsProperties jobsProperties = new JobsProperties();
+        jobsProperties = new JobsProperties();
         jobsProperties.getMemory().setDefaultJobMemory(DEFAULT_JOB_MEMORY);
         jobsProperties.getMemory().setMaxSystemMemory(MAX_SYSTEM_MEMORY);
         jobsProperties.getMemory().setMaxJobMemory(MAX_JOB_MEMORY);
@@ -70,4 +73,28 @@ public void canGetHealth() {
         Assert.assertThat(this.genieMemoryHealthIndicator.health().getStatus(), Matchers.is(Status.UP));
         Assert.assertThat(this.genieMemoryHealthIndicator.health().getStatus(), Matchers.is(Status.OUT_OF_SERVICE));
     }
+
+    /**
+     * Test to make sure invalid property values for job memory default/max don't result in exceptions.
+     */
+    @Test
+    public void canGetHealthWithInvalidJobSettings() {
+        jobsProperties.getMemory().setDefaultJobMemory(0);
+        jobsProperties.getMemory().setMaxJobMemory(0);
+        final Map<String, Object> healthDetails = genieMemoryHealthIndicator.health().getDetails();
+        Assert.assertEquals(0, healthDetails.get("availableDefaultJobCapacity"));
+        Assert.assertEquals(0, healthDetails.get("availableMaxJobCapacity"));
+    }
+
+    /**
+     * Test to make at full capacity we don't show a negative capacity.
+     */
+    @Test
+    public void nonNegativeJobsCapacity() {
+        Mockito.when(this.jobMetricsService.getUsedMemory()).thenReturn(MAX_SYSTEM_MEMORY + 100);
+        final Map<String, Object> healthDetails = genieMemoryHealthIndicator.health().getDetails();
+        Assert.assertEquals(0, healthDetails.get("availableDefaultJobCapacity"));
+        Assert.assertEquals(0, healthDetails.get("availableMaxJobCapacity"));
+    }
+
 }