work around JDK bug: if FileStore.getXXXSpace APIs return negative value, change that to Long.MAX_VALUE instead
This commit is contained in:
parent
98c39533d7
commit
ef15e1b91f
|
@ -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
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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<? extends FileAttributeView> type) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean supportsFileAttributeView(String name) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@Override
|
||||
public <V extends FileStoreAttributeView> V getFileStoreAttributeView(Class<V> 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());
|
||||
}
|
||||
}
|
|
@ -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.<Path,Integer>emptyMap());
|
||||
assertTrue(result.getDataPath().toString().contains(aPathPart));
|
||||
|
||||
nodeEnv.close();
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue