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

Display low disk watermark to be consistent with documentation #11313

Merged
merged 1 commit into from Jun 1, 2015

Conversation

mkis-
Copy link
Contributor

@mkis- mkis- commented May 22, 2015

This PR is an attempted fix for Issue #10588. I have added altered log entries, so that messages involving disk usage percentages display used disk percentages, rather than free disk percentages.

Closes #10588

@clintongormley clintongormley added >enhancement review :Core/Infra/Logging Log management and logging utilities labels May 25, 2015
@clintongormley clintongormley changed the title Issue #10588 Display low disk watermark to be consistent with documentation Display low disk watermark to be consistent with documentation May 25, 2015
@clintongormley
Copy link

@dakrone please could you review

@@ -413,7 +426,7 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
Strings.format1Decimals(freeDiskPercentage, "%"), node.nodeId());
}
return allocation.decision(Decision.NO, NAME, "less than required [%s%%] free disk on node, free: [%s%%]",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this message needs to be changed now, since the first value is the "used" value now

@dakrone
Copy link
Member

dakrone commented May 28, 2015

@mkis- I left one comment, other than that this looks good to me, let me know when that is fixed and I will merge it in!

@mkis-
Copy link
Contributor Author

mkis- commented May 30, 2015

@dakrone Thank you for your feedback. I changed the phrasing of messages around it for consistency.

DiskThresholdDecider.this.freeBytesThresholdLow, usage);
}

// Check percentage disk values
if (usage.getFreeDiskAsPercentage() < DiskThresholdDecider.this.freeDiskThresholdHigh) {
logger.warn("high disk watermark [{} free] exceeded on {}, shards will be relocated away from this node",
logger.warn("high disk watermark [{}] exceeded on {}, shards will be relocated away from this node",
Strings.format1Decimals(DiskThresholdDecider.this.freeDiskThresholdHigh, "%"), usage);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should be 100.0 - DiskThresholdDecider.this.freeDiskThresholdHigh, just like below, right?

@dakrone
Copy link
Member

dakrone commented Jun 1, 2015

@mkis- left one more comment, looks like a missed 100.0 -, can you fix and squash these to a single commit and I'll merge?

documentaiton

Change log entries, add convenience methods, unit tests

Correct allocation decision messages to display used space

Missed a word

Correct displayed threshold value
@mkis-
Copy link
Contributor Author

mkis- commented Jun 1, 2015

@dakrone Made the change and squashed the commits.

@dakrone dakrone merged commit 4261ee3 into elastic:master Jun 1, 2015
@kevinkluge kevinkluge removed the review label Jun 1, 2015
@mkis- mkis- deleted the disk-watermark branch June 1, 2015 22:18
@mkis- mkis- restored the disk-watermark branch June 1, 2015 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

doc/log enhancement: disk watermark
4 participants