[jira] [Commented] (HDDS-16217) Avoid the flatten-and-regroup in the OmKeyInfo copy constructor

["Huang Kuan Hao (Jira)" <ji...@apache.org>]

    [ 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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org