Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relax restrictions on filesystem size reporting in DiskUsage #9283

Merged
merged 1 commit into from Jan 26, 2015

Conversation

@dakrone
Copy link
Member

commented Jan 13, 2015

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, 1 is used to avoid a division-by-zero
error.

Fixes #9249
Relates to #9260

@dakrone dakrone force-pushed the dakrone:fix-9260 branch 3 times, most recently Jan 21, 2015

@dakrone

This comment has been minimized.

Copy link
Member Author

commented Jan 21, 2015

@bleskes I changed this based on our conversation, I think for now the best thing is to not change any of the settings, and program around the divide-by-zero for percentages.

I think logging the invalid values will move to #8686

@bleskes bleskes self-assigned this Jan 23, 2015

@bleskes

View changes

src/main/java/org/elasticsearch/cluster/DiskUsage.java Outdated
@@ -31,20 +31,30 @@
final long totalBytes;
final long freeBytes;

/**
* Create a new DiskUsage, if {@code totalBytes} is 0, 1 is used to avoid

This comment has been minimized.

Copy link
@bleskes

bleskes Jan 26, 2015

Member

I think this comment is no longer true?

@@ -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) {

This comment has been minimized.

Copy link
@bleskes

bleskes Jan 26, 2015

Member

I believe there is no test for this. Might be worth while adding.

@bleskes

View changes

src/test/java/org/elasticsearch/cluster/DiskUsageTests.java Outdated
@@ -34,6 +34,23 @@ public void diskUsageCalcTest() {
assertThat(du.getUsedBytes(), equalTo(60L));
assertThat(du.getTotalBytes(), equalTo(100L));

DiskUsage du2 = new DiskUsage("node1", "n1", 100, 101);

This comment has been minimized.

Copy link
@bleskes

bleskes Jan 26, 2015

Member

maybe add a comment about why we do these weird tests?

@bleskes

View changes

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

public double getFreeDiskAsPercentage() {
double freePct = 100.0 * ((double)freeBytes / totalBytes);
return freePct;
if (totalBytes == 0) {

This comment has been minimized.

Copy link
@bleskes

bleskes Jan 26, 2015

Member

maybe add a comment why we interpret totalBytes =0 as all free?

@bleskes

This comment has been minimized.

Copy link
Member

commented Jan 26, 2015

LGTM. Left some cosmetic comments , mostly around comments to explain the edge cases this covers.

@dakrone dakrone force-pushed the dakrone:fix-9260 branch Jan 26, 2015

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 #9249
Relates to #9260

@dakrone dakrone force-pushed the dakrone:fix-9260 branch to 537769c Jan 26, 2015

@dakrone dakrone merged commit 537769c into elastic:master Jan 26, 2015

1 check passed

CLA Commit author has signed the CLA
Details

@clintongormley clintongormley changed the title Relax restrictions on filesystem size reporting in DiskUsage Stats: Relax restrictions on filesystem size reporting in DiskUsage Feb 10, 2015

@dakrone dakrone deleted the dakrone:fix-9260 branch Apr 6, 2015

@clintongormley clintongormley changed the title Stats: Relax restrictions on filesystem size reporting in DiskUsage Relax restrictions on filesystem size reporting in DiskUsage Jun 8, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.