[GitHub] [ozone] sodonnel commented on a diff in pull request #3457: HDDS-6665. EC: Extend BlockReconstructedInputStreams to recover parity block buffers as well if missing

[GitBox <gi...@apache.org>]

sodonnel commented on code in PR #3457:
URL: https://github.com/apache/ozone/pull/3457#discussion_r886673789


##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockReconstructedStripeInputStream.java:
##########
@@ -411,41 +460,38 @@ private void zeroFill(ByteBuffer buf) {
    * Take the parity indexes which are already used, and the others which are
    * available, and select random indexes to meet numRequired. The resulting
    * list is sorted in ascending order of the indexes.
-   * @param locations The list of locations for all blocks in the block group/
-   * @param numRequired The number of parity chunks needed for reconstruction
-   * @return A list of indexes indicating which parity locations to read.
    */
   @SuppressWarnings("java:S2245") // no need for secure random
-  private List<Integer> selectParityIndexes(
-      DatanodeDetails[] locations, int numRequired) {
-    List<Integer> indexes = new ArrayList<>();
-    List<Integer> selected = new ArrayList<>();
-    ECReplicationConfig repConfig = getRepConfig();
-    for (int i = repConfig.getData(); i < repConfig.getRequiredNodes(); i++) {
-      if (locations[i] != null && !failedDataIndexes.contains(i)
-          && decoderInputBuffers[i] == null) {
-        indexes.add(i);
-      }
-      // If we are re-initializing, we want to make sure we are re-using any
-      // previously selected good parity indexes, as the block stream is already
-      // opened.
-      if (decoderInputBuffers[i] != null && !failedDataIndexes.contains(i)) {
+  private SortedSet<Integer> selectInternalInputs(

Review Comment:
   I wonder if it would work, by just passing fewer locations to the class? Eg, if we need to recover one index, and there 4 available where we need to read 3. If we just pass the 3 locations to the class, then it should just use them as preferred nodes as they are the only nodes available. I guess its not quite as good as it cannot fallback if there is a problem reading one of the others.
   
   But I agree that this change is complex enough already. Lets leave preferred nodes for now and add it later if needed.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org