-
Notifications
You must be signed in to change notification settings - Fork 588
HDDS-13423. Logging reason behind triggering onDemand scan on containers #8854
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
Tejaskriya
left a comment
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 working on this @sreejasahithi, please find some suggestions below.
...tainer-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
Outdated
Show resolved
Hide resolved
...vice/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OnDemandContainerScanner.java
Outdated
Show resolved
Hide resolved
...er-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerSet.java
Outdated
Show resolved
Hide resolved
Tejaskriya
left a comment
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 updating this, LGTM overall, please resolve the merge conflicts
aryangupta1998
left a comment
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 @sreejasahithi, LGTM!
adoroszlai
left a comment
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 @sreejasahithi for the patch.
I think we should:
- also include reason in "skip" messages
- make "skip" and "schedule" messages more consistent (
on-demand scan for containervs.OnDemandScan for Container)
| if (!helper.shouldScanMetadata(container)) { | ||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("Skipping on-demand scan for container {}.", container.getContainerData().getContainerID()); | ||
| } |
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.
Removed the log message here because the detailed reason is already logged in the respective methods
| }); | ||
| } else { | ||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("Skipping OnDemandScan for Container {}, Reason: {}", containerId, "Already scheduled."); |
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.
Shouldn't we log the original reason?
|
Thanks @sreejasahithi for the patch, @aryangupta1998, @Tejaskriya for the review. |
* master: (55 commits) HDDS-13525. Rename configuration property to ozone.om.compaction.service.enabled (apache#8928) HDDS-13519. Reconciliation should continue if a peer datanode is unreachable (apache#8908) HDDS-13566. Fix incorrect authorizer class in ACL documentation (apache#8931) HDDS-13084. Trigger on-demand container scan when a container moves from open to unhealthy. (apache#8904) HDDS-13432. Accelerating Namespace Usage Calculation in Recon using - Materialised Approach (apache#8797) HDDS-13557. Bump jline to 3.30.5 (apache#8920) HDDS-13556. Bump assertj-core to 3.27.4 (apache#8919) HDDS-13543. [Docs] Design doc for OM bootstrapping process with snapshots. (apache#8900) HDDS-13541. Bump sonar-maven-plugin to 5.1.0.4751 (apache#8911) HDDS-13101. Remove duplicate information in datanode list output (apache#8523) HDDS-13528. Handle null paths when the NSSummary is initializing (apache#8901) HDDS-12990. (addendum) Generate tree from metadata when it does not exist during getContainerChecksumInfo call (apache#8881) HDDS-13086. Block duplicate reconciliation requests for the same container and datanode within the datanode. (apache#8905) HDDS-12990. Generate tree from metadata when it doesn't exist during getContainerChecksumInfo call (apache#8881) HDDS-12824. Optimize container checksum read during datanode startup (apache#8604) HDDS-13522. Rename axisLabel for No. of delete request received (apache#8879) HDDS-12196. Document ozone repair cli (apache#8849) HDDS-13514. Intermittent failure in TestNSSummaryMemoryLeak (apache#8889) HDDS-13423. Log reason for triggering on-demand container scan (apache#8854) HDDS-13466. Disable flaky TestOmSnapshotFsoWithNativeLibWithLinkedBuckets ...
What changes were proposed in this pull request?
Currently there are no logs present which tell why an on-demand container scan was triggered.
This PR adds the reason for on-demand container scan at the respective locations.
What is the link to the Apache JIRA
HDDS-13423
How was this patch tested?
https://github.com/sreejasahithi/ozone/actions/runs/16463299349