[
https://issues.apache.org/jira/browse/HDDS-16161?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16161:
------------------------------
Description:
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]
was:
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.
Review:
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
> 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
> Priority: Major
>
> 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]
