Skip to content

Commit

Permalink
Relax restrictions on filesystem size reporting
Browse files Browse the repository at this point in the history
Apparently some filesystems such as ZFS and occasionally NTFS can report
filesystem usages that are negative, or above the maximum total size of
the filesystem. This relaxes the constraints on `DiskUsage` so that an
exception is not thrown.

If 0 is passed as the totalBytes, `.getFreeDiskAsPercentage()` will
always return 100.0% free (to ensure the disk threshold decider fails
open)

Fixes #9249
Relates to #9260
  • Loading branch information
dakrone committed Jan 26, 2015
1 parent 7ca2ef9 commit 537769c
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 16 deletions.
26 changes: 19 additions & 7 deletions src/main/java/org/elasticsearch/cluster/DiskUsage.java
Expand Up @@ -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() {
Expand Down
Expand Up @@ -519,6 +519,9 @@ public Decision canRemain(ShardRouting shardRouting, RoutingNode node, RoutingAl
* @return DiskUsage representing given node using the average disk usage
*/
public DiskUsage averageUsage(RoutingNode node, Map<String, DiskUsage> 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()) {
Expand All @@ -537,7 +540,9 @@ public DiskUsage averageUsage(RoutingNode node, Map<String, DiskUsage> usages) {
*/
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();
}

/**
Expand Down
34 changes: 26 additions & 8 deletions src/test/java/org/elasticsearch/cluster/DiskUsageTests.java
Expand Up @@ -34,6 +34,25 @@ public void diskUsageCalcTest() {
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
Expand All @@ -42,18 +61,17 @@ public void randomDiskUsageTest() {
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)));
}
}
}
Expand Down

0 comments on commit 537769c

Please sign in to comment.