Simplify file store

Today we go to heroic lengths to workaround bugs in the JDK or around
issues like BSD jails to get information about the underlying file
store. For example, we went to lengths to work around a JDK bug where
the file store returned would incorrectly report whether or not a path
is writable in certain situations in Windows operating
systems. Another bug prevented getting file store information on
Windows on a virtual drive on Windows. We no longer need to work
around these bugs, we could simply try to write to disk and let an I/O
exception arise if we could not write to the disk or take advantage of
the fact that these bugs are fixed in recent releases of the JDK
(e.g., the file store bug is fixed since 8u72). Additionally, we
collected information about all file stores on the system which meant
that if the user had a stale NFS mount, Elasticsearch could hang and
fail on startup if that mount point was not available. Finally, we
collected information through Lucene about whether or not a disk was a
spinning disk versus an SSD, information that we do not need since we
assume SSDs by default. This commit takes into consideration that we
simply do not need this heroic effort, we do not need information
about all file stores, and we do not need information about whether or
not a disk spins to greatly simplfy file store handling.

Relates #24402

8 files changed, 38 insertions, 205 deletions

diff --git a/core/src/main/java/org/elasticsearch/env/ESFileStore.java b/core/src/main/java/org/elasticsearch/env/ESFileStore.java
index 8ac6cf8a02..bba1e1e609 100644
--- a/core/src/main/java/org/elasticsearch/env/ESFileStore.java
+++ b/core/src/main/java/org/elasticsearch/env/ESFileStore.java
@@ -20,7 +20,6 @@
 package org.elasticsearch.env;
 
 import org.apache.lucene.util.Constants;
-import org.apache.lucene.util.IOUtils;
 import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.io.PathUtils;
 
@@ -43,25 +42,16 @@ import java.util.List;
 class ESFileStore extends FileStore {
     /** Underlying filestore */
     final FileStore in;
-    /** Cached result of Lucene's {@code IOUtils.spins} on path. */
-    final Boolean spins;
-    int majorDeviceNumber;
-    int minorDeviceNumber;
+    private int majorDeviceNumber;
+    private int minorDeviceNumber;
     
     @SuppressForbidden(reason = "tries to determine if disk is spinning")
     // TODO: move PathUtils to be package-private here instead of 
     // public+forbidden api!
-    ESFileStore(FileStore in) {
+    ESFileStore(final FileStore in) {
         this.in = in;
-        Boolean spins;
-        // Lucene's IOUtils.spins only works on Linux today:
         if (Constants.LINUX) {
             try {
-                spins = IOUtils.spins(PathUtils.get(getMountPointLinux(in)));
-            } catch (Exception e) {
-                spins = null;
-            }
-            try {
                 final List<String> lines = Files.readAllLines(PathUtils.get("/proc/self/mountinfo"));
                 for (final String line : lines) {
                     final String[] fields = line.trim().split("\\s+");
@@ -70,20 +60,21 @@ class ESFileStore extends FileStore {
                         final String[] deviceNumbers = fields[2].split(":");
                         majorDeviceNumber = Integer.parseInt(deviceNumbers[0]);
                         minorDeviceNumber = Integer.parseInt(deviceNumbers[1]);
+                        break;
                     }
                 }
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 majorDeviceNumber = -1;
                 minorDeviceNumber = -1;
             }
         } else {
-            spins = null;
+            majorDeviceNumber = -1;
+            minorDeviceNumber = -1;
         }
-        this.spins = spins;
     }
-    
+
     // these are hacks that are not guaranteed
-    private static String getMountPointLinux(FileStore store) {
+    private static String getMountPointLinux(final FileStore store) {
         String desc = store.toString();
         int index = desc.lastIndexOf(" (");
         if (index != -1) {
@@ -92,109 +83,6 @@ class ESFileStore extends FileStore {
             return desc;
         }
     }
-    
-    /** 
-     * Files.getFileStore(Path) useless here!  Don't complain, just try it yourself. 
-     */
-    @SuppressForbidden(reason = "works around the bugs")
-    static FileStore getMatchingFileStore(Path path, FileStore fileStores[]) throws IOException {       
-        if (Constants.WINDOWS) {
-            return getFileStoreWindows(path, fileStores);
-        }
-        
-        final FileStore store;
-        try {
-            store = Files.getFileStore(path);
-        } catch (IOException unexpected) {
-            // give a better error message if a filestore cannot be retrieved from inside a FreeBSD jail.
-            if (Constants.FREE_BSD) {
-                throw new IOException("Unable to retrieve mount point data for " + path +
-                                      ". If you are running within a jail, set enforce_statfs=1. See jail(8)", unexpected);
-            } else {
-                throw unexpected;
-            }
-        }
-
-        try {
-            String mount = getMountPointLinux(store);
-            FileStore sameMountPoint = null;
-            for (FileStore fs : fileStores) {
-                if (mount.equals(getMountPointLinux(fs))) {
-                    if (sameMountPoint == null) {
-                        sameMountPoint = fs;
-                    } else {
-                        // more than one filesystem has the same mount point; something is wrong!
-                        // fall back to crappy one we got from Files.getFileStore
-                        return store;
-                    }
-                }
-            }
-
-            if (sameMountPoint != null) {
-                // ok, we found only one, use it:
-                return sameMountPoint;
-            } else {
-                // fall back to crappy one we got from Files.getFileStore
-                return store;    
-            }
-        } catch (Exception e) {
-            // ignore
-        }
-
-        // fall back to crappy one we got from Files.getFileStore
-        return store;    
-    }
-    
-    /** 
-     * remove this code and just use getFileStore for windows on java 9
-     * works around https://bugs.openjdk.java.net/browse/JDK-8034057
-     */
-    @SuppressForbidden(reason = "works around https://bugs.openjdk.java.net/browse/JDK-8034057")
-    static FileStore getFileStoreWindows(Path path, FileStore fileStores[]) throws IOException {
-        assert Constants.WINDOWS;
-        
-        try {
-            return Files.getFileStore(path);
-        } catch (FileSystemException possibleBug) {
-            final char driveLetter;
-            // look for a drive letter to see if its the SUBST bug,
-            // it might be some other type of path, like a windows share
-            // if something goes wrong, we just deliver the original exception
-            try {
-                String root = path.toRealPath().getRoot().toString();
-                if (root.length() < 2) {
-                    throw new RuntimeException("root isn't a drive letter: " + root);
-                }
-                driveLetter = Character.toLowerCase(root.charAt(0));
-                if (Character.isAlphabetic(driveLetter) == false || root.charAt(1) != ':') {
-                    throw new RuntimeException("root isn't a drive letter: " + root);
-                }
-            } catch (Exception checkFailed) {
-                // something went wrong, 
-                possibleBug.addSuppressed(checkFailed);
-                throw possibleBug;
-            }
-            
-            // we have a drive letter: the hack begins!!!!!!!!
-            try {
-                // we have no choice but to parse toString of all stores and find the matching drive letter
-                for (FileStore store : fileStores) {
-                    String toString = store.toString();
-                    int length = toString.length();
-                    if (length > 3 && toString.endsWith(":)") && toString.charAt(length - 4) == '(') {
-                        if (Character.toLowerCase(toString.charAt(length - 3)) == driveLetter) {
-                            return store;
-                        }
-                    }
-                }
-                throw new RuntimeException("no filestores matched");
-            } catch (Exception weTried) {
-                IOException newException = new IOException("Unable to retrieve filestore for '" + path + "', tried matching against " + Arrays.toString(fileStores), weTried);
-                newException.addSuppressed(possibleBug);
-                throw newException;
-            }
-        }
-    }
 
     @Override
     public String name() {
@@ -263,8 +151,6 @@ class ESFileStore extends FileStore {
     @Override
     public Object getAttribute(String attribute) throws IOException {
         switch(attribute) {
-            // for the device
-            case "lucene:spins": return spins;
             // for the partition
             case "lucene:major_device_number": return majorDeviceNumber;
             case "lucene:minor_device_number": return minorDeviceNumber;
diff --git a/core/src/main/java/org/elasticsearch/env/Environment.java b/core/src/main/java/org/elasticsearch/env/Environment.java
index f431a7f646..0d30f7b576 100644
--- a/core/src/main/java/org/elasticsearch/env/Environment.java
+++ b/core/src/main/java/org/elasticsearch/env/Environment.java
@@ -27,6 +27,7 @@ import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
 
 import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.net.MalformedURLException;
 import java.net.URISyntaxException;
 import java.net.URL;
@@ -35,7 +36,9 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.function.Function;
 
@@ -97,22 +100,6 @@ public class Environment {
     /** Path to the temporary file directory used by the JDK */
     private final Path tmpFile = PathUtils.get(System.getProperty("java.io.tmpdir"));
 
-    /** List of filestores on the system */
-    private static final FileStore[] fileStores;
-
-    /**
-     * We have to do this in clinit instead of init, because ES code is pretty messy,
-     * and makes these environments, throws them away, makes them again, etc.
-     */
-    static {
-        // gather information about filesystems
-        ArrayList<FileStore> allStores = new ArrayList<>();
-        for (FileStore store : PathUtils.getDefaultFileSystem().getFileStores()) {
-            allStores.add(new ESFileStore(store));
-        }
-        fileStores = allStores.toArray(new ESFileStore[allStores.size()]);
-    }
-
     public Environment(Settings settings) {
         final Path homeFile;
         if (PATH_HOME_SETTING.exists(settings)) {
@@ -331,24 +318,8 @@ public class Environment {
         return tmpFile;
     }
 
-    /**
-     * Looks up the filestore associated with a Path.
-     * <p>
-     * This is an enhanced version of {@link Files#getFileStore(Path)}:
-     * <ul>
-     *   <li>On *nix systems, the store returned for the root filesystem will contain
-     *       the actual filesystem type (e.g. {@code ext4}) instead of {@code rootfs}.
-     *   <li>On some systems, the custom attribute {@code lucene:spins} is supported
-     *       via the {@link FileStore#getAttribute(String)} method.
-     *   <li>Only requires the security permissions of {@link Files#getFileStore(Path)},
-     *       no permissions to the actual mount point are required.
-     *   <li>Exception handling has the same semantics as {@link Files#getFileStore(Path)}.
-     *   <li>Works around https://bugs.openjdk.java.net/browse/JDK-8034057.
-     *   <li>Gives a better exception when filestore cannot be retrieved from inside a FreeBSD jail.
-     * </ul>
-     */
-    public static FileStore getFileStore(Path path) throws IOException {
-        return ESFileStore.getMatchingFileStore(path, fileStores);
+    public static FileStore getFileStore(final Path path) throws IOException {
+        return new ESFileStore(Files.getFileStore(path));
     }
 
     /**
diff --git a/core/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/core/src/main/java/org/elasticsearch/env/NodeEnvironment.java
index dec59f97f4..a24c359137 100644
--- a/core/src/main/java/org/elasticsearch/env/NodeEnvironment.java
+++ b/core/src/main/java/org/elasticsearch/env/NodeEnvironment.java
@@ -94,9 +94,6 @@ public final class NodeEnvironment  implements Closeable {
         public final Path indicesPath;
         /** Cached FileStore from path */
         public final FileStore fileStore;
-        /** Cached result of Lucene's {@code IOUtils.spins} on path.  This is a trilean value: null means we could not determine it (we are
-         *  not running on Linux, or we hit an exception trying), True means the device possibly spins and False means it does not. */
-        public final Boolean spins;
 
         public final int majorDeviceNumber;
         public final int minorDeviceNumber;
@@ -106,11 +103,9 @@ public final class NodeEnvironment  implements Closeable {
             this.indicesPath = path.resolve(INDICES_FOLDER);
             this.fileStore = Environment.getFileStore(path);
             if (fileStore.supportsFileAttributeView("lucene")) {
-                this.spins = (Boolean) fileStore.getAttribute("lucene:spins");
                 this.majorDeviceNumber = (int) fileStore.getAttribute("lucene:major_device_number");
                 this.minorDeviceNumber = (int) fileStore.getAttribute("lucene:minor_device_number");
             } else {
-                this.spins = null;
                 this.majorDeviceNumber = -1;
                 this.minorDeviceNumber = -1;
             }
@@ -136,9 +131,13 @@ public final class NodeEnvironment  implements Closeable {
         public String toString() {
             return "NodePath{" +
                     "path=" + path +
-                    ", spins=" + spins +
+                    ", indicesPath=" + indicesPath +
+                    ", fileStore=" + fileStore +
+                    ", majorDeviceNumber=" + majorDeviceNumber +
+                    ", minorDeviceNumber=" + minorDeviceNumber +
                     '}';
         }
+
     }
 
     private final NodePath[] nodePaths;
@@ -304,15 +303,6 @@ public final class NodeEnvironment  implements Closeable {
             for (NodePath nodePath : nodePaths) {
                 sb.append('\n').append(" -> ").append(nodePath.path.toAbsolutePath());
 
-                String spinsDesc;
-                if (nodePath.spins == null) {
-                    spinsDesc = "unknown";
-                } else if (nodePath.spins) {
-                    spinsDesc = "possibly";
-                } else {
-                    spinsDesc = "no";
-                }
-
                 FsInfo.Path fsPath = FsProbe.getFSInfo(nodePath);
                 sb.append(", free_space [")
                     .append(fsPath.getFree())
@@ -320,8 +310,6 @@ public final class NodeEnvironment  implements Closeable {
                     .append(fsPath.getAvailable())
                     .append("], total_space [")
                     .append(fsPath.getTotal())
-                    .append("], spins? [")
-                    .append(spinsDesc)
                     .append("], mount [")
                     .append(fsPath.getMount())
                     .append("], type [")
@@ -332,7 +320,6 @@ public final class NodeEnvironment  implements Closeable {
         } else if (logger.isInfoEnabled()) {
             FsInfo.Path totFSPath = new FsInfo.Path();
             Set<String> allTypes = new HashSet<>();
-            Set<String> allSpins = new HashSet<>();
             Set<String> allMounts = new HashSet<>();
             for (NodePath nodePath : nodePaths) {
                 FsInfo.Path fsPath = FsProbe.getFSInfo(nodePath);
@@ -343,21 +330,13 @@ public final class NodeEnvironment  implements Closeable {
                     if (type != null) {
                         allTypes.add(type);
                     }
-                    Boolean spins = fsPath.getSpins();
-                    if (spins == null) {
-                        allSpins.add("unknown");
-                    } else if (spins.booleanValue()) {
-                        allSpins.add("possibly");
-                    } else {
-                        allSpins.add("no");
-                    }
                     totFSPath.add(fsPath);
                 }
             }
 
             // Just log a 1-line summary:
-            logger.info("using [{}] data paths, mounts [{}], net usable_space [{}], net total_space [{}], spins? [{}], types [{}]",
-                nodePaths.length, allMounts, totFSPath.getAvailable(), totFSPath.getTotal(), toString(allSpins), toString(allTypes));
+            logger.info("using [{}] data paths, mounts [{}], net usable_space [{}], net total_space [{}], types [{}]",
+                nodePaths.length, allMounts, totFSPath.getAvailable(), totFSPath.getTotal(), toString(allTypes));
         }
     }
 
diff --git a/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java b/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java
index e20eb42427..6f5e9a52b4 100644
--- a/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java
+++ b/core/src/main/java/org/elasticsearch/monitor/fs/FsInfo.java
@@ -49,10 +49,6 @@ public class FsInfo implements Iterable<FsInfo.Path>, Writeable, ToXContent {
         long free = -1;
         long available = -1;
 
-        /** Uses Lucene's {@code IOUtils.spins} method to try to determine if the device backed by spinning media.
-         *  This is null if we could not determine it, true if it possibly spins, else false. */
-        Boolean spins = null;
-
         public Path() {
         }
 
@@ -74,7 +70,9 @@ public class FsInfo implements Iterable<FsInfo.Path>, Writeable, ToXContent {
             total = in.readLong();
             free = in.readLong();
             available = in.readLong();
-            spins = in.readOptionalBoolean();
+            if (in.getVersion().before(Version.V_6_0_0_alpha1_UNRELEASED)) {
+                in.readOptionalBoolean();
+            }
         }
 
         @Override
@@ -85,7 +83,9 @@ public class FsInfo implements Iterable<FsInfo.Path>, Writeable, ToXContent {
             out.writeLong(total);
             out.writeLong(free);
             out.writeLong(available);
-            out.writeOptionalBoolean(spins);
+            if (out.getVersion().before(Version.V_6_0_0_alpha1_UNRELEASED)) {
+                out.writeOptionalBoolean(null);
+            }
         }
 
         public String getPath() {
@@ -112,10 +112,6 @@ public class FsInfo implements Iterable<FsInfo.Path>, Writeable, ToXContent {
             return new ByteSizeValue(available);
         }
 
-        public Boolean getSpins() {
-            return spins;
-        }
-
         private long addLong(long current, long other) {
             if (other == -1) {
                 return current;
@@ -140,10 +136,6 @@ public class FsInfo implements Iterable<FsInfo.Path>, Writeable, ToXContent {
             total = FsProbe.adjustForHugeFilesystems(addLong(total, path.total));
             free = FsProbe.adjustForHugeFilesystems(addLong(free, path.free));
             available = FsProbe.adjustForHugeFilesystems(addLong(available, path.available));
-            if (path.spins != null && path.spins.booleanValue()) {
-                // Spinning is contagious!
-                spins = Boolean.TRUE;
-            }
         }
 
         static final class Fields {
@@ -156,7 +148,6 @@ public class FsInfo implements Iterable<FsInfo.Path>, Writeable, ToXContent {
             static final String FREE_IN_BYTES = "free_in_bytes";
             static final String AVAILABLE = "available";
             static final String AVAILABLE_IN_BYTES = "available_in_bytes";
-            static final String SPINS = "spins";
         }
 
         @Override
@@ -181,9 +172,6 @@ public class FsInfo implements Iterable<FsInfo.Path>, Writeable, ToXContent {
             if (available != -1) {
                 builder.byteSizeField(Fields.AVAILABLE_IN_BYTES, Fields.AVAILABLE, available);
             }
-            if (spins != null) {
-                builder.field(Fields.SPINS, spins.toString());
-            }
 
             builder.endObject();
             return builder;
diff --git a/core/src/main/java/org/elasticsearch/monitor/fs/FsProbe.java b/core/src/main/java/org/elasticsearch/monitor/fs/FsProbe.java
index 1fdae49a6f..8e3cd53e74 100644
--- a/core/src/main/java/org/elasticsearch/monitor/fs/FsProbe.java
+++ b/core/src/main/java/org/elasticsearch/monitor/fs/FsProbe.java
@@ -159,7 +159,6 @@ public class FsProbe extends AbstractComponent {
         fsPath.available = nodePath.fileStore.getUsableSpace();
         fsPath.type = nodePath.fileStore.type();
         fsPath.mount = nodePath.fileStore.toString();
-        fsPath.spins = nodePath.spins;
         return fsPath;
     }
 
diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy
index 6d28944680..7b1dcd788c 100644
--- a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy
+++ b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy
@@ -120,6 +120,7 @@ grant {
   permission java.io.FilePermission "/proc/sys/vm/max_map_count", "read";
 
   // io stats on Linux
+  permission java.io.FilePermission "/proc/self/mountinfo", "read";
   permission java.io.FilePermission "/proc/diskstats", "read";
 
   // control group stats on Linux
diff --git a/core/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java b/core/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java
index 0b6b14684f..9591e31b2b 100644
--- a/core/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java
+++ b/core/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java
@@ -183,7 +183,6 @@ public class NodeStatsTests extends ESTestCase {
                     assertEquals(fs.getTotal().getFree(), deserializedFs.getTotal().getFree());
                     assertEquals(fs.getTotal().getMount(), deserializedFs.getTotal().getMount());
                     assertEquals(fs.getTotal().getPath(), deserializedFs.getTotal().getPath());
-                    assertEquals(fs.getTotal().getSpins(), deserializedFs.getTotal().getSpins());
                     assertEquals(fs.getTotal().getType(), deserializedFs.getTotal().getType());
                     FsInfo.IoStats ioStats = fs.getIoStats();
                     FsInfo.IoStats deserializedIoStats = deserializedFs.getIoStats();
diff --git a/docs/reference/migration/migrate_6_0/stats.asciidoc b/docs/reference/migration/migrate_6_0/stats.asciidoc
index 633604f39a..ed70d1503c 100644
--- a/docs/reference/migration/migrate_6_0/stats.asciidoc
+++ b/docs/reference/migration/migrate_6_0/stats.asciidoc
@@ -5,3 +5,13 @@
 
 Given that store throttling has been removed, the `store` stats do not report
 `throttle_time` anymore.
+
+==== FS stats no longer reports if the disk spins
+
+Elasticsearch has defaulted to assuming that it is running on SSDs since
+the 2.x series of Elasticsearch. As such, Elasticsearch no longer needs to
+collect information from the operating system as to whether or not the
+underlying disks of each data path spin or not. While this functionality was no
+longer needed starting in the 2.x series of Elasticsearch, it was maintained in
+the filesystem section of the nodes stats APIs. This information has now been
+removed.