Guard against negative result from FileStore.getUsableSpace when picking data path for a new shard
This commit is contained in:
parent
c27237be9f
commit
98c39533d7
|
@ -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;
|
||||
|
|
|
@ -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.<Path,Integer>emptyMap());
|
||||
assertTrue(result.getDataPath().toString().contains(aPathPart));
|
||||
|
||||
nodeEnv.close();
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue