HDDS-16352. OM logs a missing lifecycle configuration as an ERROR with a stack trace - #11171
Open
henry3260 wants to merge 4 commits into
Open
HDDS-16352. OM logs a missing lifecycle configuration as an ERROR with a stack trace#11171henry3260 wants to merge 4 commits into
henry3260 wants to merge 4 commits into
Conversation
…h a stack trace OmMetadataManagerImpl#getLifecycleConfiguration logs the not-found case at debug level and then throws LIFECYCLE_CONFIGURATION_NOT_FOUND, which its own catch-all on IOException immediately reports again at error level with a stack trace. OzoneManager#getLifecycleConfiguration additionally records it as an audit read failure. A bucket without a lifecycle configuration is an expected outcome, so let that result propagate without the error log, and audit it as a completed read. Genuine failures keep logging as before. This is latent today because the only caller is GetBucketLifecycleConfiguration, which is not a hot path. HDDS-16151 adds a lifecycle lookup to HeadObject, where every request against a bucket without a configuration would hit it.
Follow-up to eaf7383, which left two gaps. OzoneManager#getLifecycleConfiguration's audit-suppression branch had no coverage; both tests exercised only OmMetadataManagerImpl. Add TestOzoneManagerGetLifecycleConfiguration, following the OmTestManagers and spy(om) harness of TestOzoneManagerListMultipartUploadsAcls, which drives both sides of the guard: a missing configuration audits as a completed read, while a genuine failure still audits as a read failure. isLifecycleConfigurationNotFound read only OMException state, and calling it from OzoneManager coupled that class to the metadata manager implementation it otherwise reaches only through OMMetadataManager. Inline the check at both call sites, matching how the rest of the repository writes this comparison, and drop the predicate along with its unit test. Behaviour is unchanged. Also join three constructs that wrapped below the 120 character limit.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts Ozone Manager lifecycle-configuration lookup behavior so that a missing bucket lifecycle configuration is treated as an expected “not found” outcome: it avoids emitting an ERROR-level stack trace in OM metadata code and audits the request as a completed read (while preserving existing behavior for genuine failures and keeping the client exception unchanged).
Changes:
- Suppress ERROR log + stack trace in
OmMetadataManagerImpl#getLifecycleConfigurationforLIFECYCLE_CONFIGURATION_NOT_FOUND. - Audit
OzoneManager#getLifecycleConfiguration“not found” as a read success (and keep auditing/logging failures for real errors). - Add unit tests covering the logging and auditing behavior for missing vs genuine-failure cases.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java | Avoid ERROR+stacktrace for the expected not-found lifecycle configuration case. |
| hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java | Treat lifecycle-config not-found as an audited completed read rather than a read failure. |
| hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java | Add regression test to ensure the not-found case does not get logged as ERROR. |
| hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerGetLifecycleConfiguration.java | Add tests to validate audit success vs failure behavior for missing vs genuine errors. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
LogCapturer implements AutoCloseable, and Log4j1Capturer only detaches its WriterAppender in stopCapturing(). Left unclosed, the appender stays on the OmMetadataManagerImpl logger for the rest of the class run and its buffer keeps accumulating. Wrap the capturer in try-with-resources.
chungen0126
self-requested a review
August 31, 2026 09:49
anuragp010
reviewed
Sep 3, 2026
anuragp010
left a comment
anuragp010
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the patch @henry3260 ! Had one minor comment.
Replace assertFalse(String#contains) with AssertJ's doesNotContain so a failure reports the captured log output instead of "expected: <true> but was: <false>", as recommended in HDDS-9951.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
OmMetadataManagerImpl#getLifecycleConfigurationlogs the not-found case at debug level and then throwsLIFECYCLE_CONFIGURATION_NOT_FOUND. SinceOMExceptionextendsIOException, the method's own catch-allimmediately reports the same condition again at error level with a full stack trace, overriding the debug
level the not-found path deliberately chose.
OzoneManager#getLifecycleConfigurationadditionally recordsit as an audit read failure.
A bucket without a lifecycle configuration is an expected outcome, not an error. This patch lets that
result propagate without the error log, and audits it as a completed read rather than a failure. Genuine
failures keep logging and auditing as before, and the exception returned to the client is unchanged.
This is latent today because the only caller is
GetBucketLifecycleConfiguration, which is not a hot path.HDDS-16151 adds a lifecycle lookup to
HeadObject, where every request against a bucket without aconfiguration would hit it.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16352
How was this patch tested?
New unit tests in
ozone-manager:TestOmMetadataManager#testGetMissingLifecycleConfigurationIsNotLoggedAsError— a missing configurationstill throws
LIFECYCLE_CONFIGURATION_NOT_FOUND, but no longer produces the error log.TestOzoneManagerGetLifecycleConfiguration— a missing configuration is audited as a completed read,while a genuine failure is still audited as a read failure.
Verified locally with
mvn -pl :ozone-manager test -Dtest='TestOmMetadataManager,TestOzoneManagerGetLifecycleConfiguration',plus
checkstyle:checkandapache-rat:checkon the module.