andreer/memory speed parsing (#4703)

* replace dummy value with optional

* handle results in the MB/s range

* simplify division

9 files changed, 65 insertions, 62 deletions

diff --git a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/commons/parser/OutputParser.java b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/commons/parser/OutputParser.java
index 5da5abd4ad2..55bf1dd0e0b 100644
--- a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/commons/parser/OutputParser.java
+++ b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/commons/parser/OutputParser.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.verification.commons.parser;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 import java.util.regex.Pattern;
 
 /**
@@ -33,12 +34,10 @@ public class OutputParser {
         return results;
     }
 
-    public static ParseResult parseSingleOutput(ParseInstructions parseInstructions, List<String> commandOutput) {
+    public static Optional<ParseResult> parseSingleOutput(ParseInstructions parseInstructions, List<String> commandOutput) {
         List<ParseResult> parseResults = parseOutput(parseInstructions, commandOutput);
-        if (parseResults.size() == 0) {
-            return new ParseResult("invalid", "invalid");
-        }
-        return parseResults.get(0);
+        if (parseResults.isEmpty()) return Optional.empty();
+        return Optional.ofNullable(parseResults.get(0));
     }
 
     private static boolean matchingSearchWord(List<String> searchWords, String searchWordCandidate) {
diff --git a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/DiskBenchmark.java b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/DiskBenchmark.java
index 7d2082f5027..a7fc5809058 100644
--- a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/DiskBenchmark.java
+++ b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/DiskBenchmark.java
@@ -10,6 +10,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Optional;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -42,22 +43,21 @@ public class DiskBenchmark implements Benchmark {
     public void doBenchmark() {
         try {
             List<String> commandOutput = commandExecutor.executeCommand(DISK_BENCHMARK_COMMAND);
-            ParseResult parseResult = parseDiskSpeed(commandOutput);
+            Optional<ParseResult> parseResult = parseDiskSpeed(commandOutput);
             setDiskSpeed(parseResult);
         } catch (IOException e) {
             logger.log(Level.WARNING, "Failed to perform disk benchmark", e);
         }
     }
 
-    ParseResult parseDiskSpeed(List<String> commandOutput) {
+    Optional<ParseResult> parseDiskSpeed(List<String> commandOutput) {
         List<String> searchWords = new ArrayList<>(Arrays.asList(KILO_BYTE_SEARCH_WORD, MEGA_BYTE_SEARCH_WORD, GIGA_BYTE_SEARCH_WORD));
         ParseInstructions parseInstructions = new ParseInstructions(SEARCH_ELEMENT_INDEX, RETURN_ELEMENT_INDEX, SPLIT_REGEX_STRING, searchWords);
         return OutputParser.parseSingleOutput(parseInstructions, commandOutput);
     }
 
-    void setDiskSpeed(ParseResult parseResult) {
-        double diskSpeedMBs = getDiskSpeedInMBs(parseResult);
-        benchmarkResults.setDiskSpeedMbs(diskSpeedMBs);
+    void setDiskSpeed(Optional<ParseResult> parseResult) {
+        benchmarkResults.setDiskSpeedMbs(parseResult.map(this::getDiskSpeedInMBs).orElse(0.0d));
     }
 
     double getDiskSpeedInMBs(ParseResult parseResult) {
diff --git a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/MemoryBenchmark.java b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/MemoryBenchmark.java
index dc7a41680ec..7a72999a1e4 100644
--- a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/MemoryBenchmark.java
+++ b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/MemoryBenchmark.java
@@ -27,7 +27,8 @@ public class MemoryBenchmark implements Benchmark {
     private static final String MEM_BENCHMARK_DELETE_FOLDER = "rm -rf RAM_test";
     private static final String MEM_BENCHMARK_WRITE_SPEED = "dd if=/dev/zero of=RAM_test/data_tmp bs=1M count=512";
     private static final String MEM_BENCHMARK_READ_SPEED = "dd if=RAM_test/data_tmp of=/dev/null bs=1M count=512";
-    private static final String READ_AND_WRITE_SEARCH_WORD = "GB/s";
+    private static final String READ_AND_WRITE_SEARCH_WORD_GB = "GB/s";
+    private static final String READ_AND_WRITE_SEARCH_WORD_MB = "MB/s";
     private static final String SPLIT_REGEX_STRING = " ";
     private static final int SEARCH_ELEMENT_INDEX = 8;
     private static final int RETURN_ELEMENT_INDEX = 7;
@@ -44,13 +45,12 @@ public class MemoryBenchmark implements Benchmark {
     public void doBenchmark() {
         try {
             setupMountPoint();
-            List<String> commandOutput = commandExecutor.executeCommand(MEM_BENCHMARK_WRITE_SPEED);
-            ParseResult parseResult = parseMemorySpeed(commandOutput);
-            parseDouble(parseResult.getValue()).ifPresent(benchmarkResults::setMemoryWriteSpeedGBs);
 
-            commandOutput = commandExecutor.executeCommand(MEM_BENCHMARK_READ_SPEED);
-            parseResult = parseMemorySpeed(commandOutput);
-            parseDouble(parseResult.getValue()).ifPresent(benchmarkResults::setMemoryReadSpeedGBs);
+            parseMemorySpeed(commandExecutor.executeCommand(MEM_BENCHMARK_WRITE_SPEED))
+                    .ifPresent(benchmarkResults::setMemoryWriteSpeedGBs);
+
+            parseMemorySpeed(commandExecutor.executeCommand(MEM_BENCHMARK_READ_SPEED))
+                    .ifPresent(benchmarkResults::setMemoryReadSpeedGBs);
         } catch (IOException e) {
             logger.log(Level.WARNING, "Failed to perform memory benchmark", e);
         } finally {
@@ -76,8 +76,18 @@ public class MemoryBenchmark implements Benchmark {
         }
     }
 
-    protected ParseResult parseMemorySpeed(List<String> commandOutput) {
-        List<String> searchWords = Collections.singletonList(READ_AND_WRITE_SEARCH_WORD);
+    protected Optional<Double> parseMemorySpeed(List<String> commandOutput) {
+        Optional<ParseResult> parseResultGb = parseMemorySpeed(commandOutput, READ_AND_WRITE_SEARCH_WORD_GB);
+        if (parseResultGb.isPresent()) return parseDouble(parseResultGb.get().getValue());
+
+        Optional<ParseResult> parseResultMb = parseMemorySpeed(commandOutput, READ_AND_WRITE_SEARCH_WORD_MB);
+        if (parseResultMb.isPresent()) return parseDouble(parseResultMb.get().getValue()).map(v -> v / 1000.0d);
+
+        return Optional.empty();
+    }
+
+    private Optional<ParseResult> parseMemorySpeed(List<String> commandOutput, String searchWord) {
+        List<String> searchWords = Collections.singletonList(searchWord);
         ParseInstructions parseInstructions = new ParseInstructions(SEARCH_ELEMENT_INDEX, RETURN_ELEMENT_INDEX, SPLIT_REGEX_STRING, searchWords);
         return OutputParser.parseSingleOutput(parseInstructions, commandOutput);
     }
diff --git a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/retrievers/DiskRetriever.java b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/retrievers/DiskRetriever.java
index 3ac863082fe..2eb6bf80bc5 100644
--- a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/retrievers/DiskRetriever.java
+++ b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/retrievers/DiskRetriever.java
@@ -70,11 +70,8 @@ public class DiskRetriever implements HardwareRetriever {
     ParseResult parseDiskType(List<String> commandOutput) throws IOException {
         List<String> searchWords = Collections.singletonList(DISK_NAME);
         ParseInstructions parseInstructions = new ParseInstructions(DISK_TYPE_SEARCH_ELEMENT_INDEX, DISK_TYPE_RETURN_ELEMENT_INDEX, DISK_TYPE_REGEX_SPLIT, searchWords);
-        ParseResult parseResult = OutputParser.parseSingleOutput(parseInstructions, commandOutput);
-        if (!parseResult.getSearchWord().equals(DISK_NAME)) {
-            throw new IOException("Parsing for disk type failed");
-        }
-        return parseResult;
+        return OutputParser.parseSingleOutput(parseInstructions, commandOutput)
+                .orElseThrow(() -> new IOException("Parsing for disk type failed"));
     }
 
     void setDiskType(ParseResult parseResult) {
diff --git a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/retrievers/MemoryRetriever.java b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/retrievers/MemoryRetriever.java
index ae3c928a0a5..2f5bbfc48dd 100644
--- a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/retrievers/MemoryRetriever.java
+++ b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/retrievers/MemoryRetriever.java
@@ -48,11 +48,8 @@ public class MemoryRetriever implements HardwareRetriever {
     ParseResult parseMemInfoFile(List<String> commandOutput) throws IOException {
         List<String> searchWords = Collections.singletonList(SEARCH_WORD);
         ParseInstructions parseInstructions = new ParseInstructions(SEARCH_ELEMENT_INDEX, RETURN_ELEMENT_INDEX, REGEX_SPLIT, searchWords);
-        ParseResult parseResult = OutputParser.parseSingleOutput(parseInstructions, commandOutput);
-        if (!parseResult.getSearchWord().matches(SEARCH_WORD)) {
-            throw new IOException("Failed to parse memory info file.");
-        }
-        return parseResult;
+        return OutputParser.parseSingleOutput(parseInstructions, commandOutput)
+                .orElseThrow(() -> new IOException("Failed to parse memory info file."));
     }
 
     void updateMemoryInfo(ParseResult parseResult) {
diff --git a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/retrievers/NetRetriever.java b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/retrievers/NetRetriever.java
index be933841688..5afdbbb1c78 100644
--- a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/retrievers/NetRetriever.java
+++ b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/verification/spec/retrievers/NetRetriever.java
@@ -95,11 +95,8 @@ public class NetRetriever implements HardwareRetriever {
     ParseResult parseInterfaceSpeed(List<String> commandOutput) throws IOException {
         List<String> searchWords = Collections.singletonList(SEARCH_WORD_INTERFACE_SPEED);
         ParseInstructions parseInstructions = new ParseInstructions(INTERFACE_SPEED_SEARCH_ELEMENT_INDEX, INTERFACE_SPEED_RETURN_ELEMENT_INDEX, INTERFACE_SPEED_REGEX_SPLIT, searchWords);
-        ParseResult parseResult = OutputParser.parseSingleOutput(parseInstructions, commandOutput);
-        if (!parseResult.getSearchWord().matches(SEARCH_WORD_INTERFACE_SPEED)) {
-            throw new IOException("Failed to parse interface speed output.");
-        }
-        return parseResult;
+        return OutputParser.parseSingleOutput(parseInstructions, commandOutput)
+                .orElseThrow(() -> new IOException("Failed to parse interface speed output."));
     }
 
     private void testPingResponse(List<ParseResult> parseResults) {
@@ -116,11 +113,8 @@ public class NetRetriever implements HardwareRetriever {
     ParseResult parsePingResponse(List<String> commandOutput) throws IOException {
         List<String> searchWords = Collections.singletonList(PING_SEARCH_WORD);
         ParseInstructions parseInstructions = new ParseInstructions(PING_SEARCH_ELEMENT_INDEX, PING_RETURN_ELEMENT_INDEX, PING_SPLIT_REGEX_STRING, searchWords);
-        ParseResult parseResult = OutputParser.parseSingleOutput(parseInstructions, commandOutput);
-        if (!parseResult.getSearchWord().matches(PING_SEARCH_WORD)) {
-            throw new IOException("Failed to parse ping output.");
-        }
-        return new ParseResult(PING_SEARCH_WORD, parseResult.getValue());
+        return new ParseResult(PING_SEARCH_WORD, OutputParser.parseSingleOutput(parseInstructions, commandOutput)
+                .orElseThrow(()->new IOException("Failed to parse ping output.")).getValue());
     }
 
     void updateHardwareInfoWithNet(List<ParseResult> parseResults) {
diff --git a/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/commons/parser/OutputParserTest.java b/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/commons/parser/OutputParserTest.java
index 060b899c099..06d34a452eb 100644
--- a/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/commons/parser/OutputParserTest.java
+++ b/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/commons/parser/OutputParserTest.java
@@ -8,6 +8,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 
 import static org.junit.Assert.assertEquals;
 
@@ -70,18 +71,17 @@ public class OutputParserTest {
     public void parseSingleOutput_should_return_first_match() {
         searchWords = Collections.singletonList(SEARCH_WORD_1);
         ParseInstructions parseInstructions = new ParseInstructions(6, 8, " ", searchWords);
-        ParseResult parseResult = OutputParser.parseSingleOutput(parseInstructions, commandOutput);
+        Optional<ParseResult> parseResult = OutputParser.parseSingleOutput(parseInstructions, commandOutput);
         ParseResult expectedParseResult = new ParseResult(SEARCH_WORD_1, RETURN_VALUE);
-        assertEquals(expectedParseResult, parseResult);
+        assertEquals(Optional.of(expectedParseResult), parseResult);
     }
 
     @Test
     public void parseSingleOutput_should_return_invalid_parseResult() {
         searchWords = Collections.singletonList("No match");
         ParseInstructions parseInstructions = new ParseInstructions(6, 8, " ", searchWords);
-        ParseResult parseResult = OutputParser.parseSingleOutput(parseInstructions, commandOutput);
-        ParseResult expectedParseResult = new ParseResult("invalid", "invalid");
-        assertEquals(expectedParseResult, parseResult);
+        Optional<ParseResult> parseResult = OutputParser.parseSingleOutput(parseInstructions, commandOutput);
+        assertEquals(Optional.empty(), parseResult);
     }
 
 }
diff --git a/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/DiskBenchmarkTest.java b/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/DiskBenchmarkTest.java
index 9bf23645316..f2dbfbdbb6f 100644
--- a/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/DiskBenchmarkTest.java
+++ b/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/DiskBenchmarkTest.java
@@ -7,6 +7,7 @@ import org.junit.Before;
 import org.junit.Test;
 
 import java.util.List;
+import java.util.Optional;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -57,31 +58,29 @@ public class DiskBenchmarkTest {
     @Test
     public void parseDiskSpeed_valid_input() throws Exception {
         List<String> mockCommandOutput = MockCommandExecutor.readFromFile(VALID_OUTPUT_FILE);
-        ParseResult parseResult = diskBenchmark.parseDiskSpeed(mockCommandOutput);
+        Optional<ParseResult> parseResult = diskBenchmark.parseDiskSpeed(mockCommandOutput);
         ParseResult expectedParseResult = new ParseResult("MB/s", "243");
-        assertEquals(expectedParseResult, parseResult);
+        assertEquals(Optional.of(expectedParseResult), parseResult);
     }
 
     @Test
     public void parseDiskSpeed_invalid_input() throws Exception {
         List<String> mockCommandOutput = MockCommandExecutor.readFromFile(INVALID_OUTPUT_FILE);
-        ParseResult parseResult = diskBenchmark.parseDiskSpeed(mockCommandOutput);
-        ParseResult expectedParseResult = new ParseResult("invalid", "invalid");
-        assertEquals(expectedParseResult, parseResult);
+        Optional<ParseResult> parseResult = diskBenchmark.parseDiskSpeed(mockCommandOutput);
+        assertEquals(Optional.empty(), parseResult);
     }
 
     @Test
     public void setDiskSpeed_valid_input() {
         ParseResult parseResult = new ParseResult("MB/s", "243");
-        diskBenchmark.setDiskSpeed(parseResult);
+        diskBenchmark.setDiskSpeed(Optional.of(parseResult));
         double expectedDiskSpeed = 243;
         assertEquals(expectedDiskSpeed, benchmarkResults.getDiskSpeedMbs(), DELTA);
     }
 
     @Test
     public void setDiskSpeed_invalid_input() {
-        ParseResult parseResult = new ParseResult("invalid", "invalid");
-        diskBenchmark.setDiskSpeed(parseResult);
+        diskBenchmark.setDiskSpeed(Optional.empty());
         double expectedDiskSpeed = 0;
         assertEquals(expectedDiskSpeed, benchmarkResults.getDiskSpeedMbs(), DELTA);
     }
diff --git a/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/MemoryBenchmarkTest.java b/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/MemoryBenchmarkTest.java
index 8d68c65b1da..e2dc19c4f4c 100644
--- a/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/MemoryBenchmarkTest.java
+++ b/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/verification/hardware/benchmarks/MemoryBenchmarkTest.java
@@ -1,12 +1,12 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.verification.hardware.benchmarks;
 
-import com.yahoo.vespa.hosted.node.verification.commons.parser.ParseResult;
 import com.yahoo.vespa.hosted.node.verification.mock.MockCommandExecutor;
 import org.junit.Before;
 import org.junit.Test;
 
 import java.util.List;
+import java.util.Optional;
 
 import static org.junit.Assert.assertEquals;
 
@@ -44,23 +44,30 @@ public class MemoryBenchmarkTest {
     }
 
     @Test
-    public void parseMemorySpeed_valid_output() throws Exception {
+    public void parseMemorySpeed_valid_output() {
         Double expectedSpeed = 12.1;
         String mockOutput = "This is a test \n the memory speed to be found is " + expectedSpeed + " GB/s";
         List<String> mockCommandOutput = commandExecutor.outputFromString(mockOutput);
-        ParseResult parseResult = memoryBenchmark.parseMemorySpeed(mockCommandOutput);
-        ParseResult expectedParseResult = new ParseResult("GB/s", expectedSpeed.toString());
-        assertEquals(expectedParseResult, parseResult);
+        Optional<Double> parseResult = memoryBenchmark.parseMemorySpeed(mockCommandOutput);
+        assertEquals(Optional.of(12.1), parseResult);
     }
 
     @Test
-    public void parseMemorySpeed_invalid_output() throws Exception {
+    public void parseMemorySpeed_slow_but_valid_output() {
+        Double expectedSpeed = 960D;
+        String mockOutput = "This is a test \n the memory speed to be found is " + expectedSpeed + " MB/s";
+        List<String> mockCommandOutput = commandExecutor.outputFromString(mockOutput);
+        Optional<Double> parseResult = memoryBenchmark.parseMemorySpeed(mockCommandOutput);
+        assertEquals(Optional.of(0.96), parseResult);
+    }
+
+    @Test
+    public void parseMemorySpeed_invalid_output() {
         List<String> mockCommandOutput = commandExecutor.outputFromString("");
-        ParseResult parseResult = memoryBenchmark.parseMemorySpeed(mockCommandOutput);
-        ParseResult expectedParseResult = new ParseResult("invalid", "invalid");
-        assertEquals(expectedParseResult, parseResult);
+        Optional<Double> parseResult = memoryBenchmark.parseMemorySpeed(mockCommandOutput);
+        assertEquals(Optional.empty(), parseResult);
         mockCommandOutput = commandExecutor.outputFromString("Exit status 1");
         parseResult = memoryBenchmark.parseMemorySpeed(mockCommandOutput);
-        assertEquals(expectedParseResult, parseResult);
+        assertEquals(Optional.empty(), parseResult);
     }
 }