-
Notifications
You must be signed in to change notification settings - Fork 593
HDDS-14446. Clarify Datanode disk space related commands and metrics #9646
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
base: master
Are you sure you want to change the base?
Conversation
| /** | ||
| * @return raw filesystem capacity (cached) for the configured volume path. | ||
| */ | ||
| public long getFsCapacity() { | ||
| return source.getCapacity(); | ||
| } | ||
|
|
||
| /** | ||
| * @return raw filesystem available space (cached) for the configured volume path. | ||
| */ | ||
| public long getFsAvailable() { | ||
| return source.getAvailable(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @siddhantsangwan for the patch.
Instead of adding these new methods in VolumeUsage, I suggest getting the same values via realUsage() in a single call, and passing that to getCurrentUsage().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch, I updated the PR.
| /** | ||
| * Simple class for grouping disk space related values. | ||
| */ | ||
| public static final class FsUsageTotals { | ||
| private final long capacity; | ||
| private final long available; | ||
| private final long used; | ||
|
|
||
| private FsUsageTotals(long capacity, long available, long used) { | ||
| this.capacity = capacity; | ||
| this.available = available; | ||
| this.used = used; | ||
| } | ||
|
|
||
| public long getCapacity() { | ||
| return capacity; | ||
| } | ||
|
|
||
| public long getAvailable() { | ||
| return available; | ||
| } | ||
|
|
||
| public long getUsed() { | ||
| return used; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I missed this earlier: can we use SpaceUsageSource.Fixed instead?
ozone/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/fs/SpaceUsageSource.java
Lines 48 to 77 in 65f0af0
| /** | |
| * A static source of space usage. Can be a point in time snapshot of a | |
| * real volume usage, or can be used for testing. | |
| */ | |
| final class Fixed implements SpaceUsageSource { | |
| private final long capacity; | |
| private final long available; | |
| private final long used; | |
| public Fixed(long capacity, long available, long used) { | |
| this.capacity = capacity; | |
| this.available = Math.max(Math.min(available, capacity - used), 0); | |
| this.used = used; | |
| } | |
| @Override | |
| public long getCapacity() { | |
| return capacity; | |
| } | |
| @Override | |
| public long getAvailable() { | |
| return available; | |
| } | |
| @Override | |
| public long getUsedSpace() { | |
| return used; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense - updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Can you please check acceptance test failure? Happened for both:
What changes were proposed in this pull request?
Please see the jira for the problem description.
Changes proposed:
capacity = fsCapacity - du.reservedThis is what the new proto message looks like:
Note
fsUsedis not reported in the heartbeat as it can be calculated where needed usingfsCapacity - fsAvailable.capacity, now we clearly differentiate by usingOzone Capacityto meanFilesystem Capacity - du.reservedetc.What's remaining (to be done in subsequent pull requests):
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14446
How was this patch tested?
Edited and added various unit tests.
The Recon overview page is still the same as before:
usageinfocommand shows the new names and the new fs stats:SCM JMX metrics:
Datanode Web UI main page (ozone-datanode-1):
Note that Total Capacity shown above should be the same as Filesystem Capacity. I've retained Total Capacity just because it was already there. We could remove it if needed.
Datanode JMX metrics (ozone-datanode-1):
PR is draft while CI runs in my fork.