Browse Source

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.
Lee Hinman 8 years ago
parent
commit
77d641216a

+ 1 - 1
core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java

@@ -137,7 +137,7 @@ public class FsInfo implements Iterable<FsInfo.Path>, Writeable, ToXContent {
         }
 
         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()) {

+ 5 - 1
core/src/main/java/org/elasticsearch/monitor/fs/FsProbe.java

@@ -136,7 +136,11 @@ public class FsProbe extends AbstractComponent {
     }
 
     /* 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;
         }

+ 24 - 0
core/src/test/java/org/elasticsearch/monitor/fs/FsProbeTests.java

@@ -40,6 +40,7 @@ import java.util.concurrent.atomic.AtomicReference;
 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 class FsProbeTests extends ESTestCase {
         }
     }
 
+    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(