Skip to content

Commit c799e19

Browse files
committed
Relax restrictions on filesystem size reporting
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 elastic#9249 Relates to elastic#9260
1 parent 73223b3 commit c799e19

File tree

3 files changed

+51
-16
lines changed

3 files changed

+51
-16
lines changed

src/main/java/org/elasticsearch/cluster/DiskUsage.java

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,32 @@ public class DiskUsage {
3131
final long totalBytes;
3232
final long freeBytes;
3333

34+
/**
35+
* Create a new DiskUsage, if {@code totalBytes} is 0, {@get getFreeDiskAsPercentage}
36+
* will always return 100.0% free
37+
*/
3438
public DiskUsage(String nodeId, String nodeName, long totalBytes, long freeBytes) {
35-
if ((totalBytes < freeBytes) || (totalBytes < 0)) {
36-
throw new IllegalStateException("Free bytes [" + freeBytes +
37-
"] cannot be less than 0 or greater than total bytes [" + totalBytes + "]");
38-
}
3939
this.nodeId = nodeId;
4040
this.nodeName = nodeName;
41-
this.totalBytes = totalBytes;
4241
this.freeBytes = freeBytes;
42+
this.totalBytes = totalBytes;
43+
}
44+
45+
public String getNodeId() {
46+
return nodeId;
47+
}
48+
49+
public String getNodeName() {
50+
return nodeName;
4351
}
4452

4553
public double getFreeDiskAsPercentage() {
46-
double freePct = 100.0 * ((double)freeBytes / totalBytes);
47-
return freePct;
54+
// We return 100.0% in order to fail "open", in that if we have invalid
55+
// numbers for the total bytes, it's as if we don't know disk usage.
56+
if (totalBytes == 0) {
57+
return 100.0;
58+
}
59+
return 100.0 * ((double)freeBytes / totalBytes);
4860
}
4961

5062
public long getFreeBytes() {

src/main/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDecider.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,9 @@ public Decision canRemain(ShardRouting shardRouting, RoutingNode node, RoutingAl
479479
* @return DiskUsage representing given node using the average disk usage
480480
*/
481481
public DiskUsage averageUsage(RoutingNode node, Map<String, DiskUsage> usages) {
482+
if (usages.size() == 0) {
483+
return new DiskUsage(node.nodeId(), node.node().name(), 0, 0);
484+
}
482485
long totalBytes = 0;
483486
long freeBytes = 0;
484487
for (DiskUsage du : usages.values()) {
@@ -497,7 +500,9 @@ public DiskUsage averageUsage(RoutingNode node, Map<String, DiskUsage> usages) {
497500
*/
498501
public double freeDiskPercentageAfterShardAssigned(DiskUsage usage, Long shardSize) {
499502
shardSize = (shardSize == null) ? 0 : shardSize;
500-
return 100.0 - (((double)(usage.getUsedBytes() + shardSize) / usage.getTotalBytes()) * 100.0);
503+
DiskUsage newUsage = new DiskUsage(usage.getNodeId(), usage.getNodeName(),
504+
usage.getTotalBytes(), usage.getFreeBytes() - shardSize);
505+
return newUsage.getFreeDiskAsPercentage();
501506
}
502507

503508
/**

src/test/java/org/elasticsearch/cluster/DiskUsageTests.java

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,25 @@ public void diskUsageCalcTest() {
3434
assertThat(du.getUsedBytes(), equalTo(60L));
3535
assertThat(du.getTotalBytes(), equalTo(100L));
3636

37+
// Test that DiskUsage handles invalid numbers, as reported by some
38+
// filesystems (ZFS & NTFS)
39+
DiskUsage du2 = new DiskUsage("node1", "n1", 100, 101);
40+
assertThat(du2.getFreeDiskAsPercentage(), equalTo(101.0));
41+
assertThat(du2.getFreeBytes(), equalTo(101L));
42+
assertThat(du2.getUsedBytes(), equalTo(-1L));
43+
assertThat(du2.getTotalBytes(), equalTo(100L));
44+
45+
DiskUsage du3 = new DiskUsage("node1", "n1", -1, -1);
46+
assertThat(du3.getFreeDiskAsPercentage(), equalTo(100.0));
47+
assertThat(du3.getFreeBytes(), equalTo(-1L));
48+
assertThat(du3.getUsedBytes(), equalTo(0L));
49+
assertThat(du3.getTotalBytes(), equalTo(-1L));
50+
51+
DiskUsage du4 = new DiskUsage("node1", "n1", 0, 0);
52+
assertThat(du4.getFreeDiskAsPercentage(), equalTo(100.0));
53+
assertThat(du4.getFreeBytes(), equalTo(0L));
54+
assertThat(du4.getUsedBytes(), equalTo(0L));
55+
assertThat(du4.getTotalBytes(), equalTo(0L));
3756
}
3857

3958
@Test
@@ -42,18 +61,17 @@ public void randomDiskUsageTest() {
4261
for (int i = 1; i < iters; i++) {
4362
long total = between(Integer.MIN_VALUE, Integer.MAX_VALUE);
4463
long free = between(Integer.MIN_VALUE, Integer.MAX_VALUE);
45-
if (free > total || total <= 0) {
46-
try {
47-
new DiskUsage("random", "random", total, free);
48-
fail("should never reach this");
49-
} catch (IllegalStateException e) {
50-
}
64+
DiskUsage du = new DiskUsage("random", "random", total, free);
65+
if (total == 0) {
66+
assertThat(du.getFreeBytes(), equalTo(free));
67+
assertThat(du.getTotalBytes(), equalTo(0L));
68+
assertThat(du.getUsedBytes(), equalTo(-free));
69+
assertThat(du.getFreeDiskAsPercentage(), equalTo(100.0));
5170
} else {
52-
DiskUsage du = new DiskUsage("random", "random", total, free);
5371
assertThat(du.getFreeBytes(), equalTo(free));
5472
assertThat(du.getTotalBytes(), equalTo(total));
5573
assertThat(du.getUsedBytes(), equalTo(total - free));
56-
assertThat(du.getFreeDiskAsPercentage(), equalTo(100.0 * ((double)free / total)));
74+
assertThat(du.getFreeDiskAsPercentage(), equalTo(100.0 * ((double) free / total)));
5775
}
5876
}
5977
}

0 commit comments

Comments
 (0)