diff --git a/src/main/java/org/elasticsearch/cluster/DiskUsage.java b/src/main/java/org/elasticsearch/cluster/DiskUsage.java index e24fa89fd19..4ff5fd53c50 100644 --- a/src/main/java/org/elasticsearch/cluster/DiskUsage.java +++ b/src/main/java/org/elasticsearch/cluster/DiskUsage.java @@ -31,20 +31,32 @@ public class DiskUsage { final long totalBytes; final long freeBytes; + /** + * Create a new DiskUsage, if {@code totalBytes} is 0, {@get getFreeDiskAsPercentage} + * will always return 100.0% free + */ public DiskUsage(String nodeId, String nodeName, long totalBytes, long freeBytes) { - if ((totalBytes < freeBytes) || (totalBytes < 0)) { - throw new IllegalStateException("Free bytes [" + freeBytes + - "] cannot be less than 0 or greater than total bytes [" + totalBytes + "]"); - } this.nodeId = nodeId; this.nodeName = nodeName; - this.totalBytes = totalBytes; this.freeBytes = freeBytes; + this.totalBytes = totalBytes; + } + + public String getNodeId() { + return nodeId; + } + + public String getNodeName() { + return nodeName; } public double getFreeDiskAsPercentage() { - double freePct = 100.0 * ((double)freeBytes / totalBytes); - return freePct; + // We return 100.0% in order to fail "open", in that if we have invalid + // numbers for the total bytes, it's as if we don't know disk usage. + if (totalBytes == 0) { + return 100.0; + } + return 100.0 * ((double)freeBytes / totalBytes); } public long getFreeBytes() { diff --git a/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDecider.java b/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDecider.java index 0c8049da870..f6cd4f4c93f 100644 --- a/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDecider.java +++ b/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDecider.java @@ -519,6 +519,9 @@ public class DiskThresholdDecider extends AllocationDecider { * @return DiskUsage representing given node using the average disk usage */ public DiskUsage averageUsage(RoutingNode node, Map usages) { + if (usages.size() == 0) { + return new DiskUsage(node.nodeId(), node.node().name(), 0, 0); + } long totalBytes = 0; long freeBytes = 0; for (DiskUsage du : usages.values()) { @@ -537,7 +540,9 @@ public class DiskThresholdDecider extends AllocationDecider { */ public double freeDiskPercentageAfterShardAssigned(DiskUsage usage, Long shardSize) { shardSize = (shardSize == null) ? 0 : shardSize; - return 100.0 - (((double)(usage.getUsedBytes() + shardSize) / usage.getTotalBytes()) * 100.0); + DiskUsage newUsage = new DiskUsage(usage.getNodeId(), usage.getNodeName(), + usage.getTotalBytes(), usage.getFreeBytes() - shardSize); + return newUsage.getFreeDiskAsPercentage(); } /** diff --git a/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java b/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java index be90831a950..62196446de8 100644 --- a/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java +++ b/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java @@ -34,6 +34,25 @@ public class DiskUsageTests extends ElasticsearchTestCase { assertThat(du.getUsedBytes(), equalTo(60L)); assertThat(du.getTotalBytes(), equalTo(100L)); + // Test that DiskUsage handles invalid numbers, as reported by some + // filesystems (ZFS & NTFS) + DiskUsage du2 = new DiskUsage("node1", "n1", 100, 101); + assertThat(du2.getFreeDiskAsPercentage(), equalTo(101.0)); + assertThat(du2.getFreeBytes(), equalTo(101L)); + assertThat(du2.getUsedBytes(), equalTo(-1L)); + assertThat(du2.getTotalBytes(), equalTo(100L)); + + DiskUsage du3 = new DiskUsage("node1", "n1", -1, -1); + assertThat(du3.getFreeDiskAsPercentage(), equalTo(100.0)); + assertThat(du3.getFreeBytes(), equalTo(-1L)); + assertThat(du3.getUsedBytes(), equalTo(0L)); + assertThat(du3.getTotalBytes(), equalTo(-1L)); + + DiskUsage du4 = new DiskUsage("node1", "n1", 0, 0); + assertThat(du4.getFreeDiskAsPercentage(), equalTo(100.0)); + assertThat(du4.getFreeBytes(), equalTo(0L)); + assertThat(du4.getUsedBytes(), equalTo(0L)); + assertThat(du4.getTotalBytes(), equalTo(0L)); } @Test @@ -42,18 +61,17 @@ public class DiskUsageTests extends ElasticsearchTestCase { for (int i = 1; i < iters; i++) { long total = between(Integer.MIN_VALUE, Integer.MAX_VALUE); long free = between(Integer.MIN_VALUE, Integer.MAX_VALUE); - if (free > total || total <= 0) { - try { - new DiskUsage("random", "random", total, free); - fail("should never reach this"); - } catch (IllegalStateException e) { - } + DiskUsage du = new DiskUsage("random", "random", total, free); + if (total == 0) { + assertThat(du.getFreeBytes(), equalTo(free)); + assertThat(du.getTotalBytes(), equalTo(0L)); + assertThat(du.getUsedBytes(), equalTo(-free)); + assertThat(du.getFreeDiskAsPercentage(), equalTo(100.0)); } else { - DiskUsage du = new DiskUsage("random", "random", total, free); assertThat(du.getFreeBytes(), equalTo(free)); assertThat(du.getTotalBytes(), equalTo(total)); assertThat(du.getUsedBytes(), equalTo(total - free)); - assertThat(du.getFreeDiskAsPercentage(), equalTo(100.0 * ((double)free / total))); + assertThat(du.getFreeDiskAsPercentage(), equalTo(100.0 * ((double) free / total))); } } }