adoroszlai commented on code in PR #4767:
URL: https://github.com/apache/ozone/pull/4767#discussion_r1204434521


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/choose/algorithms/HealthyPipelineChoosePolicy.java:
##########
@@ -43,4 +43,13 @@ public Pipeline choosePipeline(List<Pipeline> pipelineList,
     }
     return fallback;
   }
+
+  @Override
+  public int choosePipelineIndex(List<Pipeline> pipelineList,
+      PipelineRequestInformation pri) {
+    // As this class modified the passed in list, returning an index
+    // doesn't really make sense here. Throwing an exception incase any future
+    // code attempts to use this implementation.
+    throw new UnsupportedOperationException();

Review Comment:
   This policy is used in `TestWritableECContainerProvider`, so that test is 
now failing.



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PipelineChoosePolicy.java:
##########
@@ -30,8 +30,18 @@ public interface PipelineChoosePolicy {
    * Given an initial list of pipelines, return one of the pipelines.
    *
    * @param pipelineList list of pipelines.
-   * @return one of the pipelines.
+   * @return one of the pipelines or null if no pipeline can be selected.
    */
   Pipeline choosePipeline(List<Pipeline> pipelineList,
       PipelineRequestInformation pri);
+
+  /**
+   * Given a list of pipelines, return the index of the chosen pipeline.
+   * @param pipelineList List of pipelines
+   * @param pri          PipelineRequestInformation
+   * @return Index in the list of the chosen pipeline, or -1 if no pipeline
+   *         could be selected.
+   */
+  int choosePipelineIndex(List<Pipeline> pipelineList,
+      PipelineRequestInformation pri);

Review Comment:
   SCM can be configured with custom `PipelineChoosePolicy`, so we have to 
assume there may be implementations "in the wild".  In order to avoid breaking 
them, this should be added as a `default` method in the interface.  Considering 
`HealthyPipelineChoosePolicy` throws exception, such default implementation may 
not be feasible.



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