Addressing Comments

Signed-off-by: Ajay Kumar Movva <movvaam@amazon.com>
opensearch-project · Mar 5, 2024 · 4b4dd48 · 4b4dd48
1 parent a39baba
commit 4b4dd48
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 6 deletions.
diff --git a/server/src/main/java/org/opensearch/node/resource/tracker/AverageIoUsageTracker.java b/server/src/main/java/org/opensearch/node/resource/tracker/AverageIoUsageTracker.java
@@ -11,13 +11,15 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.lucene.util.Constants;
+import org.opensearch.common.ValidationException;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.monitor.fs.FsInfo.DeviceStats;
 import org.opensearch.monitor.fs.FsService;
 import org.opensearch.node.IoUsageStats;
 import org.opensearch.threadpool.ThreadPool;
 
 import java.util.HashMap;
+import java.util.Optional;
 
 /**
  * AverageIoUsageTracker tracks the IO usage by polling the FS Stats for IO metrics every (pollingInterval)
@@ -45,8 +47,9 @@ public AverageIoUsageTracker(FsService fsService, ThreadPool threadPool, TimeVal
     @Override
     public long getUsage() {
         long usage = 0;
-        if (this.preValidateFsStats()) {
-            return usage;
+        Optional<ValidationException> validationException = this.preValidateFsStats();
+        if (validationException != null && validationException.isPresent()) {
+            throw validationException.get();
         }
         // Currently even during the raid setup we have only one mount device and it is giving 0 io time from /proc/diskstats
         DeviceStats[] devicesStats = fsService.stats().getIoStats().getDevicesStats();
@@ -78,11 +81,15 @@ protected void doStart() {
         }
     }
 
-    private boolean preValidateFsStats() {
-        return fsService == null
+    public Optional<ValidationException> preValidateFsStats() {
+        ValidationException validationException = new ValidationException();
+        if (fsService == null
             || fsService.stats() == null
             || fsService.stats().getIoStats() == null
-            || fsService.stats().getIoStats().getDevicesStats() == null;
+            || fsService.stats().getIoStats().getDevicesStats() == null) {
+            validationException.addValidationError("FSService IoStats Or DeviceStats are Missing");
+        }
+        return validationException.validationErrors().isEmpty() ? Optional.empty() : Optional.of(validationException);
     }
 
     private void updateIoUsageStats() {

diff --git a/server/src/test/java/org/opensearch/node/resource/tracker/AverageUsageTrackerTests.java b/server/src/test/java/org/opensearch/node/resource/tracker/AverageUsageTrackerTests.java
@@ -8,26 +8,35 @@
 
 package org.opensearch.node.resource.tracker;
 
+import org.opensearch.common.ValidationException;
 import org.opensearch.common.unit.TimeValue;
+import org.opensearch.monitor.fs.FsInfo;
+import org.opensearch.monitor.fs.FsService;
 import org.opensearch.test.OpenSearchTestCase;
 import org.opensearch.threadpool.TestThreadPool;
 import org.opensearch.threadpool.ThreadPool;
 import org.junit.After;
 import org.junit.Before;
 
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 /**
  * Tests to validate AverageMemoryUsageTracker and AverageCpuUsageTracker implementation
  */
 public class AverageUsageTrackerTests extends OpenSearchTestCase {
     ThreadPool threadPool;
     AverageMemoryUsageTracker averageMemoryUsageTracker;
     AverageCpuUsageTracker averageCpuUsageTracker;
+    AverageIoUsageTracker averageIoUsageTracker;
 
     @Before
     public void setup() {
         threadPool = new TestThreadPool(getClass().getName());
+        FsService fsService = mock(FsService.class);
         averageMemoryUsageTracker = new AverageMemoryUsageTracker(
             threadPool,
             new TimeValue(500, TimeUnit.MILLISECONDS),
@@ -38,6 +47,12 @@ public void setup() {
             new TimeValue(500, TimeUnit.MILLISECONDS),
             new TimeValue(1000, TimeUnit.MILLISECONDS)
         );
+        averageIoUsageTracker = new AverageIoUsageTracker(
+            fsService,
+            threadPool,
+            new TimeValue(500, TimeUnit.MILLISECONDS),
+            new TimeValue(1000, TimeUnit.MILLISECONDS)
+        );
     }
 
     @After
@@ -46,14 +61,15 @@ public void cleanup() {
     }
 
     public void testBasicUsage() {
-
         assertAverageUsageStats(averageMemoryUsageTracker);
         assertAverageUsageStats(averageCpuUsageTracker);
+        assertAverageUsageStats(averageIoUsageTracker);
     }
 
     public void testUpdateWindowSize() {
         assertUpdateWindowSize(averageMemoryUsageTracker);
         assertUpdateWindowSize(averageCpuUsageTracker);
+        assertUpdateWindowSize(averageIoUsageTracker);
     }
 
     private void assertAverageUsageStats(AbstractAverageUsageTracker usageTracker) {
@@ -96,4 +112,24 @@ private void assertUpdateWindowSize(AbstractAverageUsageTracker usageTracker) {
         // ( 2 + 1 + 2 + 2 ) / 4 = 1.75
         assertEquals(1.75, usageTracker.getAverage(), 0.0);
     }
+
+    public void testPreValidationForIOTracker() {
+        Optional<ValidationException> validationException = averageIoUsageTracker.preValidateFsStats();
+        assertTrue(validationException.isPresent());
+        FsService fsService = mock(FsService.class);
+        FsInfo fsInfo = mock(FsInfo.class);
+        FsInfo.IoStats ioStats = mock(FsInfo.IoStats.class);
+        when(fsService.stats()).thenReturn(fsInfo);
+        when(fsInfo.getIoStats()).thenReturn(ioStats);
+        FsInfo.DeviceStats[] deviceStats = new FsInfo.DeviceStats[0];
+        when(fsService.stats().getIoStats().getDevicesStats()).thenReturn(deviceStats);
+        averageIoUsageTracker = new AverageIoUsageTracker(
+            fsService,
+            threadPool,
+            new TimeValue(500, TimeUnit.MILLISECONDS),
+            new TimeValue(1000, TimeUnit.MILLISECONDS)
+        );
+        validationException = averageIoUsageTracker.preValidateFsStats();
+        assertFalse(validationException.isPresent());
+    }
 }