[GitHub] [ozone] adoroszlai commented on a diff in pull request #4253: HDDS-7917. EC: ECBlockInputStream should try spare replicas on error

["adoroszlai (via GitHub)" <gi...@apache.org>]

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


##########
hadoop-hdds/client/src/test/java/org/apache/hadoop/ozone/client/io/TestECBlockInputStream.java:
##########
@@ -393,7 +394,51 @@ public void testErrorReadingBlockReportsBadLocation() throws IOException {
       BadDataLocationException e =
           assertThrows(BadDataLocationException.class, () -> ecb.read(buf));
       Assertions.assertEquals(2,
-          keyInfo.getPipeline().getReplicaIndex(e.getFailedLocation()));
+          keyInfo.getPipeline().getReplicaIndex(e.getFailedLocations().get(0)));
+    }
+  }
+
+  @Test
+  public void testNoErrorIfSpareLocationToRead() throws IOException {
+    repConfig = new ECReplicationConfig(3, 2, ECReplicationConfig.EcCodec.RS,
+        ONEMB);
+    Map<DatanodeDetails, Integer> datanodes = new LinkedHashMap<>();
+    for (int i = 1; i <= repConfig.getRequiredNodes(); i++) {
+      datanodes.put(MockDatanodeDetails.randomDatanodeDetails(), i);
+    }
+    // Add a second index = 1
+    datanodes.put(MockDatanodeDetails.randomDatanodeDetails(), 1);
+
+    BlockLocationInfo keyInfo =
+        ECStreamTestUtil.createKeyInfo(repConfig, 8 * ONEMB, datanodes);
+    try (ECBlockInputStream ecb = new ECBlockInputStream(repConfig,
+        keyInfo, true, null, null, streamFactory)) {
+      // Read a full stripe to ensure all streams are created in the stream
+      // factory
+      ByteBuffer buf = ByteBuffer.allocate(3 * ONEMB);
+      int read = ecb.read(buf);
+      Assertions.assertEquals(3 * ONEMB, read);
+      // Now make replication index 1 error on the next read but as there is a
+      // spare it should read from it with no errors
+      streamFactory.getBlockStreams().get(0).setThrowException(true);
+      buf.clear();
+      read = ecb.read(buf);
+      Assertions.assertEquals(3 * ONEMB, read);
+
+      // Now make the spare one error on the next read and we should get an
+      // error with two failed locations
+      streamFactory.getBlockStreams().get(3).setThrowException(true);

Review Comment:
   Can you please explain where index 3 is derived from?



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