-
Notifications
You must be signed in to change notification settings - Fork 0
HDDS-13758. Adding replicated size in nsummary and related changes #5
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
Conversation
Summary of ChangesHello @priyeshkaratha, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This PR introduces and integrates 'replicated size' into Apache Ozone's Recon service. It adds a Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces the replicatedSizeOfFiles field to NSSummary to track replicated data size, which is a significant enhancement for accurate disk usage reporting. The changes are well-integrated across the data model, serialization, business logic, and API endpoints. The tests have been effectively updated and parameterized to cover different replication configurations, which is great. I've identified a potential NullPointerException and a minor logging issue that should be addressed. Overall, this is a solid contribution.
| } | ||
| return totalDU; | ||
| .getNSSummary(parentId); | ||
| return nsSummary.getReplicatedSizeOfFiles(); |
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.
nsSummary can be null if getNSSummary(parentId) returns null (e.g., for an empty or unprocessed directory). This would cause a NullPointerException when nsSummary.getReplicatedSizeOfFiles() is called. A null check is needed to prevent this.
| return nsSummary.getReplicatedSizeOfFiles(); | |
| return nsSummary != null ? nsSummary.getReplicatedSizeOfFiles() : 0L; |
| if (result != ReconTaskController.ReInitializationResult.SUCCESS) { | ||
| LOG.error( | ||
| "Failed to queue reinitialization event for manual trigger (result: {}), failing the reinitialization " + | ||
| "during NSSummaryAggregatedTotalsUpgrade action, will be retried as part of syncDataFromOM " + |
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.
The log message seems to contain a copy-paste error. It mentions NSSummaryAggregatedTotalsUpgrade action, but this code is in ReplicatedSizeOfFilesUpgradeAction. This could be confusing during debugging. Please correct the action name in the log message.
| "during NSSummaryAggregatedTotalsUpgrade action, will be retried as part of syncDataFromOM " + | |
| "during ReplicatedSizeOfFilesUpgradeAction, will be retried as part of syncDataFromOM " + |
1f0a7f2 to
becb84a
Compare
What changes were proposed in this pull request?
Provide a one-liner summary of the changes in the PR Title field above.
It should be in the form of
HDDS-1234. Short summary of the change.Please describe your PR in detail:
perspective not just for the reviewer.
the Jira's description if the jira is well defined.
issue investigation, github discussion, etc.
Examples of well-written pull requests:
What is the link to the Apache JIRA
Please create an issue in ASF JIRA before opening a pull request, and you need to set the title of the pull
request which starts with the corresponding JIRA issue number. (e.g. HDDS-XXXX. Fix a typo in YYY.)
(Please replace this section with the link to the Apache JIRA)
How was this patch tested?
(Please explain how this patch was tested. Ex: unit tests, manual tests, workflow run on the fork git repo.)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this.)