HDDS-16195. Reuse computed replicated sizes in KeyDeletingService - #11028
HDDS-16195. Reuse computed replicated sizes in KeyDeletingService#11028chihsuan wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes a redundant replicated-size recomputation during pending-deletion scans in OM by reusing the replicated sizes already computed into DeletedBlock entries when building PurgedKey objects. This keeps quota accounting behavior unchanged while reducing extra traversal/allocation work in the reclaimable-key listing path.
Changes:
- Compute
PurgedKey.purgedBytesby summingDeletedBlock.getReplicatedSize()instead of callingOMKeyRequest.sumBlockLengths(). - Drop the now-unneeded
OMKeyRequestimport fromKeyManagerImpl. - Add a unit test ensuring
purgedBytesmatches the replicated-size sum of the blocks queued for deletion for a versioned key spanning multiple location groups.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java | Reuses already-computed per-block replicated sizes to derive purgedBytes, avoiding a second traversal of key location groups. |
| hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java | Adds coverage to validate purgedBytes exactly equals the sum of replicated sizes of queued DeletedBlocks. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@ashishkumar50 can you also have a look on this changes? |
ashishkumar50
left a comment
ashishkumar50
left a comment
There was a problem hiding this comment.
@chihsuan Thanks for the patch overall LGTM, Please update test code to validate in a better way.
| .mapToLong(DeletedBlock::getReplicatedSize) | ||
| .sum(); | ||
| assertEquals(blockSum, purgedKey.getPurgedBytes(), | ||
| "purged bytes must cover exactly the blocks queued for deletion"); |
There was a problem hiding this comment.
This is exactly the implementation formula used in test which will always be success. Instead assert the actual key size to validate whether the purge key yields the correct size.
What changes were proposed in this pull request?
While building the reclaimable-key list,
KeyManagerImplalready computes and stores each block's replicated size inDeletedBlock. It then traverses the key locations again throughOMKeyRequest.sumBlockLengths()to recompute the same values forPurgedKey.This change calculates
purgedBytesfrom theDeletedBlockvalues already produced. It avoids another traversal of the key-location groups, their flattened-list allocations, and repeatedQuotaUtil.getReplicatedSize()calls.The resulting quota value and behavior are unchanged.
This complements HDDS-16183: that change improves
sumBlockLengths()for its remaining callers, while this change removes a redundant call from the pending-deletion scan. The patches touch different files and can be merged independently.What is the link to the Apache JIRA?
https://issues.apache.org/jira/browse/HDDS-16195
How was this patch tested?
Added
checkPurgedBytesMatchBlocksQueuedForDeletion, which creates a versioned key spanning two location groups and verifies thatpurgedBytesequals the replicated-size sum of the blocks queued for deletion.The targeted test passed:
TestKeyDeletingService$Normal#checkPurgedBytesMatchBlocksQueuedForDeletionCheckstyle reports no errors for the changed files.
Generated-by: Claude Code (Opus 5)