diff --git a/core/src/main/java/org/elasticsearch/env/ESFileStore.java b/core/src/main/java/org/elasticsearch/env/ESFileStore.java index 8051c522d03..8ac6cf8a02a 100644 --- a/core/src/main/java/org/elasticsearch/env/ESFileStore.java +++ b/core/src/main/java/org/elasticsearch/env/ESFileStore.java @@ -213,17 +213,32 @@ class ESFileStore extends FileStore { @Override public long getTotalSpace() throws IOException { - return in.getTotalSpace(); + long result = in.getTotalSpace(); + if (result < 0) { + // see https://bugs.openjdk.java.net/browse/JDK-8162520: + result = Long.MAX_VALUE; + } + return result; } @Override public long getUsableSpace() throws IOException { - return in.getUsableSpace(); + long result = in.getUsableSpace(); + if (result < 0) { + // see https://bugs.openjdk.java.net/browse/JDK-8162520: + result = Long.MAX_VALUE; + } + return result; } @Override public long getUnallocatedSpace() throws IOException { - return in.getUnallocatedSpace(); + long result = in.getUnallocatedSpace(); + if (result < 0) { + // see https://bugs.openjdk.java.net/browse/JDK-8162520: + result = Long.MAX_VALUE; + } + return result; } @Override 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 1b160d5fad5..9ec28d6d777 100644 --- a/core/src/main/java/org/elasticsearch/index/shard/ShardPath.java +++ b/core/src/main/java/org/elasticsearch/index/shard/ShardPath.java @@ -26,6 +26,7 @@ import org.elasticsearch.env.ShardLock; import org.elasticsearch.index.IndexSettings; import java.io.IOException; +import java.math.BigInteger; import java.nio.file.FileStore; import java.nio.file.Files; import java.nio.file.Path; @@ -171,9 +172,9 @@ public final class ShardPath { dataPath = env.resolveCustomLocation(indexSettings, shardId); statePath = env.nodePaths()[0].resolve(shardId); } else { - long totFreeSpace = 0; + BigInteger totFreeSpace = BigInteger.ZERO; for (NodeEnvironment.NodePath nodePath : env.nodePaths()) { - totFreeSpace += Math.max(0, nodePath.fileStore.getUsableSpace()); + totFreeSpace = totFreeSpace.add(BigInteger.valueOf(nodePath.fileStore.getUsableSpace())); } // TODO: this is a hack!! We should instead keep track of incoming (relocated) shards since we know @@ -181,25 +182,24 @@ public final class ShardPath { // Very rough heuristic of how much disk space we expect the shard will use over its lifetime, the max of current average // shard size across the cluster and 5% of the total available free space on this node: - long estShardSizeInBytes = Math.max(avgShardSizeInBytes, (long) (totFreeSpace/20.0)); + BigInteger estShardSizeInBytes = BigInteger.valueOf(avgShardSizeInBytes).max(totFreeSpace.divide(BigInteger.valueOf(20))); // TODO - do we need something more extensible? Yet, this does the job for now... final NodeEnvironment.NodePath[] paths = env.nodePaths(); NodeEnvironment.NodePath bestPath = null; - long maxUsableBytes = Long.MIN_VALUE; + BigInteger maxUsableBytes = BigInteger.valueOf(-1); for (NodeEnvironment.NodePath nodePath : paths) { FileStore fileStore = nodePath.fileStore; - // 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()); + BigInteger usableBytes = BigInteger.valueOf(fileStore.getUsableSpace()); + assert usableBytes.compareTo(BigInteger.ZERO) >= 0; // Deduct estimated reserved bytes from usable space: Integer count = dataPathToShardCount.get(nodePath.path); if (count != null) { - usableBytes = Math.subtractExact(usableBytes, Math.multiplyExact(estShardSizeInBytes, count)); + usableBytes = usableBytes.subtract(estShardSizeInBytes.multiply(BigInteger.valueOf(count))); } - if (usableBytes > maxUsableBytes) { + if (usableBytes.compareTo(maxUsableBytes) > 0) { maxUsableBytes = usableBytes; bestPath = nodePath; } diff --git a/core/src/test/java/org/elasticsearch/env/ESFileStoreTests.java b/core/src/test/java/org/elasticsearch/env/ESFileStoreTests.java new file mode 100644 index 00000000000..a0e33979a03 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/env/ESFileStoreTests.java @@ -0,0 +1,95 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.env; + +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.nio.file.FileStore; +import java.nio.file.attribute.FileAttributeView; +import java.nio.file.attribute.FileStoreAttributeView; + +public class ESFileStoreTests extends ESTestCase { + static class MockFileStore extends FileStore { + + public long usableSpace = -1; + + @Override + public String type() { + return "mock"; + } + + @Override + public String name() { + return "mock"; + } + + @Override + public String toString() { + return "mock"; + } + + @Override + public boolean isReadOnly() { + return false; + } + + @Override + public long getTotalSpace() throws IOException { + return usableSpace*3; + } + + @Override + public long getUsableSpace() throws IOException { + return usableSpace; + } + + @Override + public long getUnallocatedSpace() throws IOException { + return usableSpace*2; + } + + @Override + public boolean supportsFileAttributeView(Class type) { + return false; + } + + @Override + public boolean supportsFileAttributeView(String name) { + return false; + } + + @Override + public V getFileStoreAttributeView(Class type) { + return null; + } + + @Override + public Object getAttribute(String attribute) throws IOException { + return null; + } + } + + public void testNegativeSpace() throws Exception { + FileStore store = new ESFileStore(new MockFileStore()); + assertEquals(Long.MAX_VALUE, store.getUsableSpace()); + assertEquals(Long.MAX_VALUE, store.getTotalSpace()); + assertEquals(Long.MAX_VALUE, store.getUnallocatedSpace()); + } +} 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 2698de2d840..f1515cd559b 100644 --- a/core/src/test/java/org/elasticsearch/index/shard/NewPathForShardTests.java +++ b/core/src/test/java/org/elasticsearch/index/shard/NewPathForShardTests.java @@ -211,31 +211,4 @@ 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(); - } }