Chi-Hsuan Huang created HDDS-16195:
--------------------------------------
Summary: Avoid recomputing block replicated sizes in the
KeyDeletingService reclaimable-key scan
Key: HDDS-16195
URL: https://issues.apache.org/jira/browse/HDDS-16195
Project: Apache Ozone
Issue Type: Improvement
Components: OM
Reporter: Chi-Hsuan Huang
h3. Problem
{{KeyManagerImpl}} builds the reclaimable key list for {{KeyDeletingService}}
in {{KeyManagerImpl.java:853\-868}}. For every key that passes the reclaim
filter it walks the key's blocks twice and computes the replicated size of each
block twice:
{code:java}
List deletedBlocks = info.getKeyLocationVersions\(\).stream\(\)
.flatMap\(versionLocations \->
versionLocations.getLocationList\(\).stream\(\)
.map\(b \-> new DeletedBlock\(
new BlockID\(b.getContainerID\(\), b.getLocalID\(\)\),
b.getLength\(\),
QuotaUtil.getReplicatedSize\(b.getLength\(\),
info.getReplicationConfig\(\)\),
QuotaUtil.getSizePerReplica\(b.getLength\(\),
info.getReplicationConfig\(\)\)
\)\)\).collect\(Collectors.toList\(\)\);
...
reclaimableKeys.put\(blockGroupName,
new PurgedKey\(info.getVolumeName\(\), info.getBucketName\(\), bucketId,
keyBlocks, kv.getKey\(\), OMKeyRequest.sumBlockLengths\(info\),
info.isDeletedKeyCommitted\(\)\)\);
{code}
The first traversal already stores the per\-block replicated size in
{{DeletedBlock}}
\({{hadoop\-hdds/framework/src/main/java/org/apache/hadoop/ozone/common/DeletedBlock.java}}\),
which exposes it through {{getReplicatedSize\(\)}}.
The second traversal is {{OMKeyRequest.sumBlockLengths\(info\)}}
\({{OMKeyRequest.java:887}}\), which sums
{{QuotaUtil.getReplicatedSize\(locationInfo.getLength\(\),
info.getReplicationConfig\(\)\)}} over the same blocks.
Both traversals sit in the same filter branch and both start from
{{info.getKeyLocationVersions\(\)}}, so they visit an identical set of block
objects and apply identical arithmetic. The value {{sumBlockLengths}}
recomputes is therefore already in hand:
{code:java}
deletedBlocks.stream\(\).mapToLong\(DeletedBlock::getReplicatedSize\).sum\(\)
{code}
That removes, per reclaimable key: one full traversal of every block, one
{{getLocationList\(\)}} flattening copy per version group, and one
{{QuotaUtil.getReplicatedSize}} call per block.
h3. Why this is separate from HDDS\-16183
HDDS\-16183 proposes swapping {{getLocationList\(\)}} for
{{getLocationLists\(\)}} inside {{sumBlockLengths}}, which benefits every
caller of that method. It does not address the duplicate traversal here,
because the duplication is independent of which accessor is used. Conversely,
the change proposed here removes the {{sumBlockLengths}} call from this loop
altogether.
The two changes touch different files, do not conflict, and can land in either
order.
h3. Scope
This loop runs per pending\-delete key on every {{KeyDeletingService}}
iteration, so the duplicated work repeats across the whole scan.
h3. Notes
Behavior is unchanged: the block set and the arithmetic are identical, so the
resulting quota value is the same. Existing key deletion and quota tests cover
it.
No benchmark has been run. RocksDB iteration and {{OmKeyInfo}} deserialization
plausibly dominate this loop, so this should be treated as removal of provably
redundant work rather than a claimed speedup. Unlike an allocation\-shaped
observation, the redundancy here is verifiable by reading the code: the two
traversals demonstrably cover the same blocks.
Pinned source commit 4766aa8609. Analysis assisted by AI tooling \(Claude Code,
Opus 5\).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
