szetszwo commented on code in PR #10454:
URL: https://github.com/apache/ozone/pull/10454#discussion_r3468780980


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java:
##########
@@ -380,37 +364,91 @@ void removeContainerFromPipeline(PipelineID pipelineID, 
ContainerID containerID)
    */
   Pipeline updatePipelineState(PipelineID pipelineID, PipelineState state)
       throws PipelineNotFoundException {
-    Objects.requireNonNull(pipelineID, "Pipeline Id cannot be null");
     Objects.requireNonNull(state, "Pipeline LifeCycleState cannot be null");
 
-    final Pipeline pipeline = getPipeline(pipelineID);
+    final PipelineInfo info = getPipeline(pipelineID);
+    final Pipeline pipeline = info.getPipeline();
     // Return the old pipeline if updating same state
     if (pipeline.getPipelineState() == state) {
       LOG.debug("CurrentState and NewState are the same, return from " +
           "updatePipelineState directly.");
       return pipeline;
     }
-    Pipeline updatedPipeline = pipelineMap.compute(pipelineID,
-        (id, p) -> pipeline.toBuilder().setState(state).build());
+    final Pipeline updated = pipeline.toBuilder().setState(state).build();
+    PipelineInfo oldInfo = getPipeline(pipelineID);
+
+    PipelineInfo newInfo = new PipelineInfo(updated);
+
+    for (ContainerID cid : oldInfo.copyContainers()) {
+      newInfo.addContainer(cid);
+    }
+
+    pipelineMap.put(pipelineID, newInfo);
 
     List<Pipeline> pipelineList =
         query2OpenPipelines.get(pipeline.getReplicationConfig());
 
-    if (updatedPipeline.getPipelineState() == PipelineState.OPEN) {
+    if (updated.getPipelineState() == PipelineState.OPEN) {
       // for transition to OPEN state add pipeline to query2OpenPipelines
       if (pipelineList == null) {
         pipelineList = new ArrayList<>();
         query2OpenPipelines.put(pipeline.getReplicationConfig(), pipelineList);
       }
-      pipelineList.add(updatedPipeline);
+      pipelineList.add(updated);
     } else {
       // for transition from OPEN to CLOSED state remove pipeline from
       // query2OpenPipelines
       if (pipelineList != null) {
         pipelineList.remove(pipeline);
       }
     }
-    return updatedPipeline;
+    return updated;
   }
 
+  static class PipelineInfo {
+    private final Pipeline pipeline;
+    private final NavigableSet<ContainerID> containers = new TreeSet<>();
+
+    PipelineInfo(Pipeline pipeline) {
+      this.pipeline = pipeline;
+    }
+
+    Pipeline getPipeline() {
+      return pipeline;
+    }
+
+    NavigableSet<ContainerID> getContainers() {
+      return new TreeSet<>(containers);
+    }
+    
+    NavigableSet<ContainerID> copyContainers() {
+      return new TreeSet<>(containers);
+    }

Review Comment:
   Why these two methods are the same?



-- 
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