diff --git a/core/src/main/java/org/elasticsearch/index/shard/ShardPath.java b/core/src/main/java/org/elasticsearch/index/shard/ShardPath.java index be0d51bd2b6..1b160d5fad5 100644 --- a/core/src/main/java/org/elasticsearch/index/shard/ShardPath.java +++ b/core/src/main/java/org/elasticsearch/index/shard/ShardPath.java @@ -173,7 +173,7 @@ public final class ShardPath { } else { long totFreeSpace = 0; for (NodeEnvironment.NodePath nodePath : env.nodePaths()) { - totFreeSpace += nodePath.fileStore.getUsableSpace(); + totFreeSpace += Math.max(0, nodePath.fileStore.getUsableSpace()); } // TODO: this is a hack!! We should instead keep track of incoming (relocated) shards since we know @@ -189,12 +189,15 @@ public final class ShardPath { long maxUsableBytes = Long.MIN_VALUE; for (NodeEnvironment.NodePath nodePath : paths) { FileStore fileStore = nodePath.fileStore; - long usableBytes = fileStore.getUsableSpace(); + + // Apparently, FileStore.getUsableSpace() can sometimes be negative, even possibly Long.MIN_VALUE, which can lead to NPE + // below since we would fail to set bestPath: + long usableBytes = Math.max(0, fileStore.getUsableSpace()); // Deduct estimated reserved bytes from usable space: Integer count = dataPathToShardCount.get(nodePath.path); if (count != null) { - usableBytes -= estShardSizeInBytes * count; + usableBytes = Math.subtractExact(usableBytes, Math.multiplyExact(estShardSizeInBytes, count)); } if (usableBytes > maxUsableBytes) { maxUsableBytes = usableBytes; diff --git a/core/src/test/java/org/elasticsearch/index/shard/NewPathForShardTests.java b/core/src/test/java/org/elasticsearch/index/shard/NewPathForShardTests.java index f1515cd559b..2698de2d840 100644 --- a/core/src/test/java/org/elasticsearch/index/shard/NewPathForShardTests.java +++ b/core/src/test/java/org/elasticsearch/index/shard/NewPathForShardTests.java @@ -211,4 +211,31 @@ public class NewPathForShardTests extends ESTestCase { nodeEnv.close(); } + + public void testNegativeUsableSpace() throws Exception { + Path path = PathUtils.get(createTempDir().toString()); + + // Use 1 data path: + String[] paths = new String[] {path.resolve("a").toString()}; + + Settings settings = Settings.builder() + .put(Environment.PATH_HOME_SETTING.getKey(), path) + .putArray(Environment.PATH_DATA_SETTING.getKey(), paths).build(); + NodeEnvironment nodeEnv = new NodeEnvironment(settings, new Environment(settings)); + + // Make sure all our mocking above actually worked: + NodePath[] nodePaths = nodeEnv.nodePaths(); + assertEquals(1, nodePaths.length); + + assertEquals("mocka", nodePaths[0].fileStore.name()); + + // Path a is confused about its free space: + aFileStore.usableSpace = -Long.MIN_VALUE; + + ShardId shardId = new ShardId("index", "_na_", 0); + ShardPath result = ShardPath.selectNewPathForShard(nodeEnv, shardId, INDEX_SETTINGS, 100, Collections.emptyMap()); + assertTrue(result.getDataPath().toString().contains(aPathPart)); + + nodeEnv.close(); + } }