[
https://issues.apache.org/jira/browse/HDDS-10446?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrei Mikhalev updated HDDS-10446:
-----------------------------------
Description:
There is a base class
[`org.apache.hadoop.hdds.scm.node.states.Node2ObjectsMap|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ObjectsMap.java#L40]
for
[Node2PipelineMap|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java#L37]
and
[Node2ContainerMap|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ContainerMap.java#L37].
It is redundant because of:
1. {{Node2ContainerMap}} is used only in tests
2. {{Node2PipelineMap}} doesn't need inheritance at all
Moreover {{Node2PipelineMap}} contains {{ConcurrentHashMap<UUID,
Set<PipelineID>>}} and 2 {{synchronized}} methods:
* {{void addPipeline(Pipeline pipeline)}} -> [compute if absent for concurrent
hash
map|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java#L68]
* {{void removePipeline(Pipeline pipeline)}} -> [compute if present for
concurrent hash
map|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java#L76]
I suppose {{synchronized}} is redundant here for these two methods.
Proposed changes:
1. Remove {{org.apache.hadoop.hdds.scm.node.states.Node2ObjectsMap}} class
2. Move {{Node2ContainerMap}} to tests
3. Don't use {{synchronized}} for {{Node2PipelineMap::addPipeline}} and
{{Node2PipelineMap::removePipeline}}
GitHub discussions [here|https://github.com/apache/ozone/discussions/6294]
was:
There is a base class
[`org.apache.hadoop.hdds.scm.node.states.Node2ObjectsMap|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ObjectsMap.java#L40]
for
[Node2PipelineMap|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java#L37]
and
[Node2ContainerMap|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ContainerMap.java#L37].
It is redundant because of:
1. {{Node2ContainerMap}} is used only in tests
2. {{Node2PipelineMap}} doesn't need inheritance at all
Moreover {{Node2PipelineMap}} contains {{ConcurrentHashMap<UUID,
Set<PipelineID>>}} and 2 {{synchronized}} methods:
* {{void addPipeline(Pipeline pipeline)}} -> [compute if absent for concurrent
hash
map|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java#L68]
* {{void removePipeline(Pipeline pipeline)}} -> [compute if present for
concurrent hash
map|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java#L76]
I suppose `synchronized` is redundant here for these two methods.
Proposed changes:
1. Remove {{org.apache.hadoop.hdds.scm.node.states.Node2ObjectsMap}} class
2. Move {{Node2ContainerMap}} to tests
3. Don't use {{synchronized}} for {{Node2PipelineMap::addPipeline}} and
{{Node2PipelineMap::removePipeline}}
GitHub discussions [here|https://github.com/apache/ozone/discussions/6294]
> Refactoring Node2ObjectsMap, Node2PipelineMap, Node2ContainerMap
> ----------------------------------------------------------------
>
> Key: HDDS-10446
> URL: https://issues.apache.org/jira/browse/HDDS-10446
> Project: Apache Ozone
> Issue Type: Improvement
> Components: SCM
> Affects Versions: 1.4.0
> Reporter: Andrei Mikhalev
> Assignee: Andrei Mikhalev
> Priority: Minor
> Labels: pull-request-available
>
> There is a base class
> [`org.apache.hadoop.hdds.scm.node.states.Node2ObjectsMap|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ObjectsMap.java#L40]
> for
> [Node2PipelineMap|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java#L37]
> and
> [Node2ContainerMap|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ContainerMap.java#L37].
> It is redundant because of:
> 1. {{Node2ContainerMap}} is used only in tests
> 2. {{Node2PipelineMap}} doesn't need inheritance at all
> Moreover {{Node2PipelineMap}} contains {{ConcurrentHashMap<UUID,
> Set<PipelineID>>}} and 2 {{synchronized}} methods:
> * {{void addPipeline(Pipeline pipeline)}} -> [compute if absent for
> concurrent hash
> map|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java#L68]
> * {{void removePipeline(Pipeline pipeline)}} -> [compute if present for
> concurrent hash
> map|https://github.com/apache/ozone/blob/master/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java#L76]
> I suppose {{synchronized}} is redundant here for these two methods.
> Proposed changes:
> 1. Remove {{org.apache.hadoop.hdds.scm.node.states.Node2ObjectsMap}} class
> 2. Move {{Node2ContainerMap}} to tests
> 3. Don't use {{synchronized}} for {{Node2PipelineMap::addPipeline}} and
> {{Node2PipelineMap::removePipeline}}
> GitHub discussions [here|https://github.com/apache/ozone/discussions/6294]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
