hanishakoneru commented on a change in pull request #2947:
URL: https://github.com/apache/ozone/pull/2947#discussion_r782493800



##########
File path: 
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java
##########
@@ -355,6 +371,8 @@ public void stopOzoneManager(String omNodeId) {
     private static final String SCM_NODE_ID_PREFIX = "scmNode-";
     private List<StorageContainerManager> activeSCMs = new ArrayList<>();
     private List<StorageContainerManager> inactiveSCMs = new ArrayList<>();
+    private final ReservedPorts omPorts = new ReservedPorts(3);
+    private final ReservedPorts scmPorts = new ReservedPorts(3);

Review comment:
       Can we add a comment here on what the 3 reserved ports per node are for?

##########
File path: 
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java
##########
@@ -1055,4 +1087,53 @@ public void exitSystem(int status, String message, 
Logger log)
       throw new IOException(message);
     }
   }
+
+  /**
+   * Reserves a number of ports for services.
+   */
+  private static class ReservedPorts {
+
+    private final Queue<ServerSocket> allPorts = new LinkedList<>();
+    private final Map<String, List<ServerSocket>> assignedPorts =
+        new HashMap<>();
+    private final int portsPerNode;
+
+    ReservedPorts(int portsPerNode) {
+      this.portsPerNode = portsPerNode;
+    }
+
+    public void reserve(int nodes) {
+      Preconditions.checkState(allPorts.isEmpty());
+      allPorts.addAll(reservePorts(portsPerNode * nodes));
+    }
+
+    public PrimitiveIterator.OfInt assign(String id) {
+      Preconditions.checkState(allPorts.size() >= portsPerNode);
+      List<ServerSocket> nodePorts = new LinkedList<>();
+      for (int i = 0; i < portsPerNode; i++) {
+        nodePorts.add(allPorts.remove());
+      }
+      assignedPorts.put(id, nodePorts);
+      LOG.debug("assign ports for {}: {}", id, nodePorts);
+
+      return 
nodePorts.stream().mapToInt(ServerSocket::getLocalPort).iterator();
+    }
+
+    public void release(String id) {

Review comment:
       Can you please add Javadocs to the functions in ReservedPorts.

##########
File path: 
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java
##########
@@ -673,25 +708,19 @@ private void initOMHAConfig(int basePort) throws 
IOException {
           OMConfigKeys.OZONE_OM_NODES_KEY, omServiceId);
       List<String> omNodeIds = new ArrayList<>();
 
-      int port = basePort;
+      omPorts.reserve(numOfOMs);
+      ReservedPorts omRpcPorts = new ReservedPorts(1);
+      omRpcPorts.reserve(numOfOMs);

Review comment:
       Why do Rpc Ports need to be reserved separately?

##########
File path: 
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java
##########
@@ -1022,7 +1054,7 @@ public StorageContainerManager 
getStorageContainerManager() {
     return getStorageContainerManagers().get(0);
   }
 
-  private List<Integer> getFreePortList(int size) {
+  private static List<Integer> getFreePortList(int size) {

Review comment:
       This function is redundant now.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



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

Reply via email to