[
https://issues.apache.org/jira/browse/HDDS-16161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18112322#comment-18112322
]
Chi-Hsuan Huang edited comment on HDDS-16161 at 9/7/26 12:53 PM:
-----------------------------------------------------------------
I audited all callers of OMKeyRequest.getBucketInfo():
* 17 mutating classes still need copy-and-publish updates.
1 was already fixed by HDDS-16117.
4 are read-only/safe.
I’m verifying which mutations have reachable failure paths. OMKeyPurgeRequest
currently appears preventive rather than a confirmed leak.
I’ll create a PR with incremental commits per caller or related pair, with
corresponding tests.
was (Author: JIRAUSER313111):
I audited all callers of {*}getBucketInfo(){*}. 11 have a confirmed unsafe
mutation path like HDDS-16117:
* 4 in key/dir/file create (namespace quota charged before the request can
fail)
* 4 in key delete/commit (quota release mutated before failure paths)
* 2 in purge requests (mutated across loop iterations, copied only in finally)
* 1 in expired multipart abort
Another 5 share the same unprotected pattern, but the current code has no
reachable failure path (only unchecked cache writes follow the mutation), and 5
are read-only/safe.
I'll create a PR with an incremental commit per call site.
> Audit OMKeyRequest#getBucketInfo callers for unsafe cache mutation
> ------------------------------------------------------------------
>
> Key: HDDS-16161
> URL: https://issues.apache.org/jira/browse/HDDS-16161
> Project: Apache Ozone
> Issue Type: Improvement
> Reporter: Siyao Meng
> Assignee: Chi-Hsuan Huang
> Priority: Critical
>
> h2. Background
> HDDS-16117 fixes a namespace quota leak caused by mutating the live cached
> {{OmBucketInfo}} returned by {{OMKeyRequest.getBucketInfo()}} before a
> multipart complete request was known to succeed.
> During the review of PR 10984, it was noted that {{getBucketInfo()}} was used
> in 25 places. The remaining callers should be reviewed for the same unsafe
> mutation pattern. {{OMDirectoryCreateRequest}} was identified as one
> potential case.
> Ref: [https://github.com/apache/ozone/pull/10984#pullrequestreview-4912799793]
> h2. Problem
> {{OMKeyRequest.getBucketInfo()}} returns the cached {{OmBucketInfo}} instance
> by reference. A caller that modifies this object before all fallible
> processing has completed can change shared cache state even if the request
> later fails and does not persist its response.
> Documenting this behavior helps, but does not prevent another caller from
> accidentally introducing the same type of cache leak.
> h2. Scope
> * Audit the remaining {{OMKeyRequest.getBucketInfo()}} callers.
> * Classify each caller as read-only or mutating.
> * Fix confirmed unsafe callers by modifying a copy and publishing it only on
> the successful request path.
> * Add a guardrail that makes the distinction between read-only access and
> update access explicit.
> * Add regression tests for confirmed failure-path cache leaks.
> Possible guardrails include separate read-only and update getters, with the
> update getter returning a copy. A read-only interface or view could provide
> stronger compile-time protection, but the final design should follow existing
> OM request patterns and avoid unnecessary changes to read-only callers.
> h2. Acceptance criteria
> * All current callers are reviewed.
> * Confirmed failure-path mutations of the cached bucket are corrected.
> * Updating callers operate on request-owned state until the request succeeds.
> * Tests demonstrate that failed requests do not change cached bucket
> metadata without a corresponding durable update.
> * Successful quota accounting and persistence behavior remain unchanged.
> h2. Related work
> * HDDS-16117
> * [https://github.com/apache/ozone/pull/10984]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
