This is an automated email from the ASF dual-hosted git repository.

adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git


The following commit(s) were added to refs/heads/master by this push:
     new f7a421b27b HDDS-10446. Refactor Node2ObjectsMap, Node2PipelineMap, 
Node2ContainerMap (#6303)
f7a421b27b is described below

commit f7a421b27bde7cce83ba18a35b0658b9ab106ba5
Author: Andrei Mikhalev <[email protected]>
AuthorDate: Tue Mar 5 12:32:30 2024 +0300

    HDDS-10446. Refactor Node2ObjectsMap, Node2PipelineMap, Node2ContainerMap 
(#6303)
---
 .../hdds/scm/node/states/Node2ContainerMap.java    |  92 -------------------
 .../hdds/scm/node/states/Node2PipelineMap.java     |  28 ++++--
 .../hadoop/hdds/scm/container/MockNodeManager.java |   1 -
 .../hdds/scm/container/Node2ContainerMap.java}     | 101 +++++++++++----------
 .../TestNode2ContainerMap.java                     |   6 +-
 5 files changed, 72 insertions(+), 156 deletions(-)

diff --git 
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ContainerMap.java
 
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ContainerMap.java
deleted file mode 100644
index c0f46f15fe..0000000000
--- 
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ContainerMap.java
+++ /dev/null
@@ -1,92 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with this
- * work for additional information regarding copyright ownership.  The ASF
- * licenses this file to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations 
under
- * the License.
- *
- */
-
-package org.apache.hadoop.hdds.scm.node.states;
-
-import java.util.HashSet;
-import java.util.Set;
-import java.util.UUID;
-
-import org.apache.hadoop.hdds.scm.container.ContainerID;
-import org.apache.hadoop.hdds.scm.exceptions.SCMException;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
-import static org.apache.hadoop.hdds.scm.exceptions.SCMException.ResultCodes
-    .NO_SUCH_DATANODE;
-
-/**
- * This data structure maintains the list of containers that is on a datanode.
- * This information is built from the DN container reports.
- */
-public class Node2ContainerMap extends Node2ObjectsMap<ContainerID> {
-
-  /**
-   * Constructs a Node2ContainerMap Object.
-   */
-  public Node2ContainerMap() {
-    super();
-  }
-
-  /**
-   * Returns null if there no containers associated with this datanode ID.
-   *
-   * @param datanode - UUID
-   * @return Set of containers or Null.
-   */
-  public Set<ContainerID> getContainers(UUID datanode) {
-    return getObjects(datanode);
-  }
-
-  /**
-   * Insert a new datanode into Node2Container Map.
-   *
-   * @param datanodeID   -- Datanode UUID
-   * @param containerIDs - List of ContainerIDs.
-   */
-  @Override
-  public void insertNewDatanode(UUID datanodeID, Set<ContainerID> containerIDs)
-      throws SCMException {
-    super.insertNewDatanode(datanodeID, containerIDs);
-  }
-
-  /**
-   * Updates the Container list of an existing DN.
-   *
-   * @param datanodeID - UUID of DN.
-   * @param containers - Set of Containers tht is present on DN.
-   * @throws SCMException - if we don't know about this datanode, for new DN
-   *                        use addDatanodeInContainerMap.
-   */
-  public void setContainersForDatanode(UUID datanodeID,
-      Set<ContainerID> containers) throws SCMException {
-    Preconditions.checkNotNull(datanodeID);
-    Preconditions.checkNotNull(containers);
-    if (dn2ObjectMap
-        .computeIfPresent(datanodeID, (k, v) -> new HashSet<>(containers))
-        == null) {
-      throw new SCMException("No such datanode", NO_SUCH_DATANODE);
-    }
-  }
-
-  @VisibleForTesting
-  @Override
-  public int size() {
-    return dn2ObjectMap.size();
-  }
-}
diff --git 
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java
 
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java
index 6533cb8076..35107829f8 100644
--- 
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java
+++ 
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java
@@ -18,10 +18,14 @@
 
 package org.apache.hadoop.hdds.scm.node.states;
 
+import jakarta.annotation.Nonnull;
 import org.apache.hadoop.hdds.protocol.DatanodeDetails;
 import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
 import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
 
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
@@ -34,11 +38,13 @@ import java.util.concurrent.ConcurrentHashMap;
  * <p>TODO: this information needs to be regenerated from pipeline reports
  * on SCM restart
  */
-public class Node2PipelineMap extends Node2ObjectsMap<PipelineID> {
+public class Node2PipelineMap {
+  private final Map<UUID, Set<PipelineID>> dn2PipelineMap = new 
ConcurrentHashMap<>();
 
-  /** Constructs a Node2PipelineMap Object. */
+  /**
+   * Constructs a Node2PipelineMap Object.
+   */
   public Node2PipelineMap() {
-    super();
   }
 
   /**
@@ -47,17 +53,19 @@ public class Node2PipelineMap extends 
Node2ObjectsMap<PipelineID> {
    * @param datanode - UUID
    * @return Set of pipelines or Null.
    */
-  public Set<PipelineID> getPipelines(UUID datanode) {
-    return getObjects(datanode);
+  public Set<PipelineID> getPipelines(@Nonnull UUID datanode) {
+    final Set<PipelineID> s = dn2PipelineMap.get(datanode);
+    return s != null ? new HashSet<>(s) : Collections.emptySet();
   }
 
   /**
    * Return 0 if there are no pipelines associated with this datanode ID.
+   *
    * @param datanode - UUID
    * @return Number of pipelines or 0.
    */
   public int getPipelinesCount(UUID datanode) {
-    return getObjects(datanode).size();
+    return getPipelines(datanode).size();
   }
 
   /**
@@ -65,18 +73,18 @@ public class Node2PipelineMap extends 
Node2ObjectsMap<PipelineID> {
    *
    * @param pipeline Pipeline to be added
    */
-  public synchronized void addPipeline(Pipeline pipeline) {
+  public void addPipeline(Pipeline pipeline) {
     for (DatanodeDetails details : pipeline.getNodes()) {
       UUID dnId = details.getUuid();
-      dn2ObjectMap.computeIfAbsent(dnId, k -> ConcurrentHashMap.newKeySet())
+      dn2PipelineMap.computeIfAbsent(dnId, k -> ConcurrentHashMap.newKeySet())
           .add(pipeline.getId());
     }
   }
 
-  public synchronized void removePipeline(Pipeline pipeline) {
+  public void removePipeline(Pipeline pipeline) {
     for (DatanodeDetails details : pipeline.getNodes()) {
       UUID dnId = details.getUuid();
-      dn2ObjectMap.computeIfPresent(dnId,
+      dn2PipelineMap.computeIfPresent(dnId,
           (k, v) -> {
             v.remove(pipeline.getId());
             return v;
diff --git 
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java
 
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java
index 84f3684ab7..21c3f1c9a8 100644
--- 
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java
+++ 
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java
@@ -42,7 +42,6 @@ import org.apache.hadoop.hdds.scm.node.DatanodeInfo;
 import org.apache.hadoop.hdds.scm.node.DatanodeUsageInfo;
 import org.apache.hadoop.hdds.scm.node.NodeManager;
 import org.apache.hadoop.hdds.scm.node.NodeStatus;
-import org.apache.hadoop.hdds.scm.node.states.Node2ContainerMap;
 import org.apache.hadoop.hdds.scm.node.states.Node2PipelineMap;
 import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
 import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
diff --git 
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ObjectsMap.java
 
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/Node2ContainerMap.java
similarity index 63%
rename from 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ObjectsMap.java
rename to 
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/Node2ContainerMap.java
index 5269a7aaeb..507eb75c5d 100644
--- 
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ObjectsMap.java
+++ 
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/Node2ContainerMap.java
@@ -16,37 +16,47 @@
  *
  */
 
-package org.apache.hadoop.hdds.scm.node.states;
+package org.apache.hadoop.hdds.scm.container;
 
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
+import jakarta.annotation.Nonnull;
 import org.apache.hadoop.hdds.scm.exceptions.SCMException;
+import org.apache.hadoop.hdds.scm.node.states.ReportResult;
 
-import java.util.UUID;
-import java.util.Set;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeSet;
-import java.util.HashSet;
-import java.util.Collections;
-
+import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 
 import static 
org.apache.hadoop.hdds.scm.exceptions.SCMException.ResultCodes.DUPLICATE_DATANODE;
+import static 
org.apache.hadoop.hdds.scm.exceptions.SCMException.ResultCodes.NO_SUCH_DATANODE;
 
 /**
  * This data structure maintains the list of containers that is on a datanode.
  * This information is built from the DN container reports.
  */
-public class Node2ObjectsMap<T> {
+class Node2ContainerMap {
+  private final Map<UUID, Set<ContainerID>> dn2ContainerMap = new 
ConcurrentHashMap<>();
 
-  @SuppressWarnings("visibilitymodifier")
-  protected final Map<UUID, Set<T>> dn2ObjectMap;
 
   /**
    * Constructs a Node2ContainerMap Object.
    */
-  public Node2ObjectsMap() {
-    dn2ObjectMap = new ConcurrentHashMap<>();
+  Node2ContainerMap() {
+    super();
+  }
+
+  /**
+   * Returns null if there no containers associated with this datanode ID.
+   *
+   * @param datanode - UUID
+   * @return Set of containers or Null.
+   */
+  public @Nonnull Set<ContainerID> getContainers(@Nonnull UUID datanode) {
+    final Set<ContainerID> s = dn2ContainerMap.get(datanode);
+    return s != null ? new HashSet<>(s) : Collections.emptySet();
   }
 
   /**
@@ -56,9 +66,8 @@ public class Node2ObjectsMap<T> {
    * @param datanodeID - UUID of the Datanode.
    * @return True if this is tracked, false if this map does not know about it.
    */
-  public boolean isKnownDatanode(UUID datanodeID) {
-    Preconditions.checkNotNull(datanodeID);
-    return dn2ObjectMap.containsKey(datanodeID);
+  public boolean isKnownDatanode(@Nonnull UUID datanodeID) {
+    return dn2ContainerMap.containsKey(datanodeID);
   }
 
   /**
@@ -67,15 +76,10 @@ public class Node2ObjectsMap<T> {
    * @param datanodeID   -- Datanode UUID
    * @param containerIDs - List of ContainerIDs.
    */
-  @VisibleForTesting
-  public void insertNewDatanode(UUID datanodeID, Set<T> containerIDs)
+  public void insertNewDatanode(@Nonnull UUID datanodeID, @Nonnull 
Set<ContainerID> containerIDs)
       throws SCMException {
-    Preconditions.checkNotNull(containerIDs);
-    Preconditions.checkNotNull(datanodeID);
-    if (dn2ObjectMap.putIfAbsent(datanodeID, new HashSet<>(containerIDs))
-        != null) {
-      throw new SCMException("Node already exists in the map",
-          DUPLICATE_DATANODE);
+    if (dn2ContainerMap.putIfAbsent(datanodeID, new HashSet<>(containerIDs)) 
!= null) {
+      throw new SCMException("Node already exists in the map", 
DUPLICATE_DATANODE);
     }
   }
 
@@ -84,32 +88,15 @@ public class Node2ObjectsMap<T> {
    *
    * @param datanodeID - Datanode ID.
    */
-  @VisibleForTesting
-  public void removeDatanode(UUID datanodeID) {
-    Preconditions.checkNotNull(datanodeID);
-    dn2ObjectMap.computeIfPresent(datanodeID, (k, v) -> null);
+  public void removeDatanode(@Nonnull UUID datanodeID) {
+    dn2ContainerMap.computeIfPresent(datanodeID, (k, v) -> null);
   }
 
-  /**
-   * Returns null if there no containers associated with this datanode ID.
-   *
-   * @param datanode - UUID
-   * @return Set of containers or Null.
-   */
-  Set<T> getObjects(UUID datanode) {
-    Preconditions.checkNotNull(datanode);
-    final Set<T> s = dn2ObjectMap.get(datanode);
-    return s != null ? new HashSet<>(s) : Collections.emptySet();
-  }
-
-  public ReportResult.ReportResultBuilder<T> newBuilder() {
+  public @Nonnull ReportResult.ReportResultBuilder<ContainerID> newBuilder() {
     return new ReportResult.ReportResultBuilder<>();
   }
 
-  public ReportResult<T> processReport(UUID datanodeID, Set<T> objects) {
-    Preconditions.checkNotNull(datanodeID);
-    Preconditions.checkNotNull(objects);
-
+  public @Nonnull ReportResult<ContainerID> processReport(@Nonnull UUID 
datanodeID, @Nonnull Set<ContainerID> objects) {
     if (!isKnownDatanode(datanodeID)) {
       return newBuilder()
           .setStatus(ReportResult.ReportStatus.NEW_DATANODE_FOUND)
@@ -118,11 +105,11 @@ public class Node2ObjectsMap<T> {
     }
 
     // Conditions like Zero length containers should be handled by removeAll.
-    Set<T> currentSet = dn2ObjectMap.get(datanodeID);
-    TreeSet<T> newObjects = new TreeSet<>(objects);
+    Set<ContainerID> currentSet = dn2ContainerMap.get(datanodeID);
+    TreeSet<ContainerID> newObjects = new TreeSet<>(objects);
     newObjects.removeAll(currentSet);
 
-    TreeSet<T> missingObjects = new TreeSet<>(currentSet);
+    TreeSet<ContainerID> missingObjects = new TreeSet<>(currentSet);
     missingObjects.removeAll(objects);
 
     if (newObjects.isEmpty() && missingObjects.isEmpty()) {
@@ -159,8 +146,22 @@ public class Node2ObjectsMap<T> {
         .build();
   }
 
-  @VisibleForTesting
+  /**
+   * Updates the Container list of an existing DN.
+   *
+   * @param datanodeID - UUID of DN.
+   * @param containers - Set of Containers tht is present on DN.
+   * @throws SCMException - if we don't know about this datanode, for new DN
+   *                        use addDatanodeInContainerMap.
+   */
+  public void setContainersForDatanode(@Nonnull UUID datanodeID, @Nonnull 
Set<ContainerID> containers)
+      throws SCMException {
+    if (dn2ContainerMap.computeIfPresent(datanodeID, (k, v) -> new 
HashSet<>(containers)) == null) {
+      throw new SCMException("No such datanode", NO_SUCH_DATANODE);
+    }
+  }
+
   public int size() {
-    return dn2ObjectMap.size();
+    return dn2ContainerMap.size();
   }
 }
diff --git 
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/states/TestNode2ContainerMap.java
 
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestNode2ContainerMap.java
similarity index 99%
rename from 
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/states/TestNode2ContainerMap.java
rename to 
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestNode2ContainerMap.java
index 0aab0aeca8..92e0a2c494 100644
--- 
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/states/TestNode2ContainerMap.java
+++ 
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestNode2ContainerMap.java
@@ -17,10 +17,10 @@
  *
  */
 
-package org.apache.hadoop.hdds.scm.node.states;
+package org.apache.hadoop.hdds.scm.container;
 
-import org.apache.hadoop.hdds.scm.container.ContainerID;
 import org.apache.hadoop.hdds.scm.exceptions.SCMException;
+import org.apache.hadoop.hdds.scm.node.states.ReportResult;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -32,9 +32,9 @@ import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 
 /**
  * Test classes for Node2ContainerMap.


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to