[
https://issues.apache.org/jira/browse/HDDS-16217?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105501#comment-18105501
]
Huang Kuan Hao commented on HDDS-16217:
---------------------------------------
Closing as Won't Fix.
The current flatten-and-rebuild in the OmKeyInfo copy constructor is
intentional, not accidental overhead.
OmKeyInfo.Builder(OmKeyInfo) passes getLocationList() into the
OmKeyLocationInfoGroup(long, List, boolean) constructor, which rebuilds a new
HashMap with a fresh ArrayList per createVersion. That gives the copy its own
independent inner lists — only the OmKeyLocationInfo element references are
shared.
The proposed shortcut (passing new HashMap<>(getLocationVersionMap()) to the
Map constructor) is a shallow copy: the map spine is new, but the inner List
values are the same objects as the source group. So the copy and the source
would share their inner block lists.
That breaks the copy-constructor contract. OmKeyInfo.copyObject() (new
Builder(this).build()) is the deep copy used across OM for cache isolation and
snapshot denormalization, and its results are routinely mutated in place — e.g.
OmKeyLocationInfoGroup.appendNewBlocks does locationList.add(...) on the inner
list, reached from OMKeyCreateRequest, OMAllocateBlockRequest, and the FSO
variants. With shared inner lists, appending blocks to a copy would also append
them to the cached/committed source, corrupting it.
Any behavior-preserving alternative would have to rebuild the map with a fresh
ArrayList per version, which is exactly what the List constructor already does
— so there is no allocation saving to be had. The path is also a per-copy
operation over small per-version block lists, not a measured hotspot.
> Avoid the flatten-and-regroup in the OmKeyInfo copy constructor
> ---------------------------------------------------------------
>
> Key: HDDS-16217
> URL: https://issues.apache.org/jira/browse/HDDS-16217
> Project: Apache Ozone
> Issue Type: Sub-task
> Reporter: Huang Kuan Hao
> Assignee: Huang Kuan Hao
> Priority: Major
>
> OmKeyInfo.Builder(OmKeyInfo) copies each version by passing getLocationList()
> to the (long, List, boolean) constructor, which immediately regroups the
> flattened list back into a version map — so the flatten is pure overhead:
> obj.keyLocationVersions.forEach(keyLocationVersion ->
> this.omKeyLocationInfoGroups.add(
> new OmKeyLocationInfoGroup(keyLocationVersion.getVersion(),
> keyLocationVersion.getLocationList(),
> keyLocationVersion.isMultipartKey())));
> Fix: pass new HashMap<>(keyLocationVersion.getLocationVersionMap()) to the
> map-taking constructor — this skips the flatten-and-regroup while keeping the
> map spine independent from the source. Behavior unchanged.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
