You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "chihsuan (via GitHub)" <gi...@apache.org> on 2026/08/16 03:54:33 UTC

[PR] HDDS-16183. Avoid the per-group list copy in OMKeyRequest.sumBlockLengths [ozone]

chihsuan opened a new pull request, #11026:
URL: https://github.com/apache/ozone/pull/11026

   ## What changes were proposed in this pull request?
   
   **Problem.** `OMKeyRequest.sumBlockLengths` reaches a key's blocks through `OmKeyLocationInfoGroup.getLocationList()`, which flattens the version map into a fresh `ArrayList` on every call. That accessor carries an explicit warning from its author ("Use this expensive method only when absolutely needed! ... Use getLocationLists() instead"), added in HDDS-5384 together with the cheap alternative. `sumBlockLengths` predates that change, so it was written before `getLocationLists()` existed and was never updated.
   
   **Fix.** Iterate `getLocationLists()` with one more nested loop. Both accessors expose the same block objects, so the sum is identical and no behavior changes.
   
   This is a cleanup that follows the accessor's documented contract, not a claimed speedup. An isolated measurement of the method (details in the Jira) shows the new form allocates nothing where the current form pays roughly 470 B per call even for a single-block key, but that saving has not been shown to be observable in the `KeyDeletingService` scan end to end, where RocksDB reads and `OmKeyInfo` deserialization dominate.
   
   HDDS-16195 tracks a separate defect at the `KeyManagerImpl` caller and does not overlap with this change.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-16183
   
   ## How was this patch tested?
   
   - No new test added. The change swaps one accessor for another over the same block objects, and existing tests already assert the resulting quota values through the callers of `sumBlockLengths`:
     - `TestOMKeyCommitRequest#testCommitWithHsyncIncrementalUsages` asserts the bucket `usedBytes` deltas (300 / 600 / 1000) across incremental commits, which sums older key versions.
     - `TestOMDirectoriesPurgeRequestAndResponse#testValidateAndUpdateCacheCheckQuota` and `#testValidateAndUpdateCacheQuotaBucketRecreated` assert bucket `usedBytes` after a directory purge.
   - Ran the OM key delete, commit and directory purge request suites: `TestOMKeyDeleteRequest`, `TestOMKeyDeleteRequestWithFSO`, `TestOMKeysDeleteRequest`, `TestOMKeysDeleteRequestWithFSO`, `TestOMKeyCommitRequest`, `TestOMKeyCommitRequestWithFSO`, `TestOMDirectoriesPurgeRequestAndResponse`. 88 tests, all passing.
   - Ran `checkstyle.sh` and `author.sh` clean.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-16183. Avoid the per-group list copy in OMKeyRequest.sumBlockLengths [ozone]

Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #11026:
URL: https://github.com/apache/ozone/pull/11026#issuecomment-5315295226

   I've created a Jira ticket to rename `OmKeyLocationInfoGroup#getLocationList` to `OmKeyLocationInfoGroup#createLocationList` to prevent misuse. I briefly checked the references and found quite a few instances of it being misused, so I just wanted to mention it here to let everyone know.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-16183. Avoid the per-group list copy in OMKeyRequest.sumBlockLengths [ozone]

Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #11026:
URL: https://github.com/apache/ozone/pull/11026#issuecomment-5314926867

   Merged. Thanks @chihsuan for working on this, @yandrey321 for the review.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-16183. Avoid the per-group list copy in OMKeyRequest.sumBlockLengths [ozone]

Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 merged PR #11026:
URL: https://github.com/apache/ozone/pull/11026


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org