adoroszlai commented on code in PR #5560:
URL: https://github.com/apache/ozone/pull/5560#discussion_r1398479130
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateManager.java:
##########
@@ -65,8 +65,7 @@ void updatePipelineState(HddsProtos.PipelineID
pipelineIDProto,
void addContainerToPipeline(
PipelineID pipelineID,
- ContainerID containerID
- ) throws IOException;
+ ContainerID containerID, boolean checkPipelineClosed) throws IOException;
Review Comment:
Instead of changing the interface, how about adding it as a member variable,
set via `PipelineStateManagerImpl.Builder` here:
https://github.com/apache/ozone/blob/086e303f145e1f7a4ded5f403e0255c898fef721/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconPipelineManager.java#L78-L84
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java:
##########
@@ -97,19 +97,24 @@ void addPipeline(Pipeline pipeline) throws IOException {
*
* @param pipelineID - PipelineID of the pipeline to which container is added
* @param containerID - ContainerID of the container to add
+ * @param checkPipelineClosed - check if pipeline is closed
Review Comment:
I think `rejectClosedPipeline` would be a better name to describe the
purpose of this parameter. `check...` is ambiguous, it may as well indicate
that the method only accepts closed pipelines...
(Or flip the meaning of the flag and use `allowClosedPipeline`.)
--
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]
