-
Notifications
You must be signed in to change notification settings - Fork 566
HDDS-10298. Replace estimated count with actual count while listing openkeys. #9043
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
@smengcl can you please review the code changes? |
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.
@priyeshkaratha given some comment
...-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/om/ListOpenFilesSubCommand.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
Outdated
Show resolved
Hide resolved
Thanks @sumitagrawl for the review. Addressed your review comments. |
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.
Pull Request Overview
This PR replaces the estimated count from RocksDB's estimateCount
API with an actual count by iterating through the relevant open key tables. The change improves accuracy when listing open files by providing exact counts instead of estimates.
- Replaced estimated count with actual count calculation for open files listing
- Refactored the count method to accept bucket layout and prefix parameters for targeted counting
- Updated CLI output to remove "(est.)" annotation since counts are now accurate
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
OmMetadataManagerImpl.java | Replaced estimated count method with actual iteration-based counting |
OMMetadataManager.java | Removed the public interface for estimated count method |
ListOpenFilesSubCommand.java | Updated CLI output formatting to reflect accurate counts |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
Show resolved
Hide resolved
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
Show resolved
Hide resolved
try (TableIterator<String, String> | ||
keyValueTableIterator = getOpenKeyTable(bucketLayout).keyIterator(prefix)) { | ||
while (keyValueTableIterator.hasNext()) { | ||
count += 1; | ||
keyValueTableIterator.next(); | ||
} | ||
} catch (UncheckedIOException e) { |
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.
Can we measure how fast it can iterate over 10 million open keys?
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.
lgtm
What changes were proposed in this pull request?
As part of this change, the use of the RocksDB estimateCount API for displaying estimated counts has been removed. Instead iterating over either openKeyTable or openFileTable based on the prefix given and calculate the total count.
What is the link to the Apache JIRA
HDDS-10298
How was this patch tested?
Tested locally using command line.