Handle long overflow when adding paths' totals

From #23093, we fixed the issue where a filesystem can be so large that it overflows and returns a negative number. However, there is another issue when adding a path as a sub-path to another `FsInfo.Path` object, when adding the totals the values can still overflow. This adds the same safety to return `Long.MAX_VALUE` instead of the negative number, as well as a test exercising the logic.
elastic · Feb 22, 2017 · 77d6412 · 77d6412
1 parent 0f88f21
commit 77d6412
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 2 deletions.
diff --git a/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java b/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java
@@ -137,7 +137,7 @@ private double addDouble(double current, double other) {
         }
 
         public void add(Path path) {
-            total = addLong(total, path.total);
+            total = FsProbe.adjustForHugeFilesystems(addLong(total, path.total));
             free = addLong(free, path.free);
             available = addLong(available, path.available);
             if (path.spins != null && path.spins.booleanValue()) {

diff --git a/core/src/main/java/org/elasticsearch/monitor/fs/FsProbe.java b/core/src/main/java/org/elasticsearch/monitor/fs/FsProbe.java
@@ -136,7 +136,11 @@ List<String> readProcDiskStats() throws IOException {
     }
 
     /* See: https://bugs.openjdk.java.net/browse/JDK-8162520 */
-    private static long adjustForHugeFilesystems(long bytes) {
+    /**
+     * Take a large value intended to be positive, and if it has overflowed,
+     * return {@code Long.MAX_VALUE} instead of a negative number.
+     */
+    static long adjustForHugeFilesystems(long bytes) {
         if (bytes < 0) {
             return Long.MAX_VALUE;
         }

diff --git a/core/src/test/java/org/elasticsearch/monitor/fs/FsProbeTests.java b/core/src/test/java/org/elasticsearch/monitor/fs/FsProbeTests.java
@@ -40,6 +40,7 @@
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.lessThan;
 import static org.hamcrest.Matchers.isEmptyOrNullString;
 import static org.hamcrest.Matchers.not;
 
@@ -91,6 +92,29 @@ public void testFsInfo() throws IOException {
         }
     }
 
+    public void testFsInfoOverflow() throws Exception {
+        FsInfo.Path pathStats = new FsInfo.Path("/foo/bar", null,
+                randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
+
+        // While not overflowing, keep adding
+        FsInfo.Path pathToAdd = new FsInfo.Path("/foo/baz", null,
+                randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
+        while ((pathStats.total + pathToAdd.total) > 0) {
+            // Add itself as a path, to increase the total bytes until it overflows
+            logger.info("--> adding {} bytes to {}, will be: {}", pathToAdd.total, pathStats.total, pathToAdd.total + pathStats.total);
+            pathStats.add(pathToAdd);
+            pathToAdd = new FsInfo.Path("/foo/baz", null,
+                randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
+        }
+
+        logger.info("--> adding {} bytes to {}, will be: {}", pathToAdd.total, pathStats.total, pathToAdd.total + pathStats.total);
+        assertThat(pathStats.total + pathToAdd.total, lessThan(0L));
+        pathStats.add(pathToAdd);
+
+        // Even after overflowing, it should not be negative
+        assertThat(pathStats.total, greaterThan(0L));
+    }
+
     public void testIoStats() {
         final AtomicReference<List<String>> diskStats = new AtomicReference<>();
         diskStats.set(Arrays.asList(