[GitHub] [hadoop] steveloughran commented on a diff in pull request #4273: HADOOP-18107 Adding scale test for vectored reads for large file

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

steveloughran commented on code in PR #4273:
URL: https://github.com/apache/hadoop/pull/4273#discussion_r870087061


##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java:
##########
@@ -1102,7 +1101,15 @@ public static void validateFileContent(byte[] concat, byte[][] bytes) {
                 mismatch);
   }
 
-  public static void validateVectoredReadResult(List<FileRange> fileRanges, byte[] DATASET)
+  /**
+   * Utility to validate vectored read results.
+   * @param fileRanges input ranges.
+   * @param originalData original data.
+   * @throws ExecutionException any exception.

Review Comment:
   the future io stuff will unwrap execution exceptions. so remove the catch/fail there, just let it all get passed up to the test case



##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java:
##########
@@ -1115,8 +1122,9 @@ public static void validateVectoredReadResult(List<FileRange> fileRanges, byte[]
     for (FileRange res : fileRanges) {
       CompletableFuture<ByteBuffer> data = res.getData();
       try {
-        ByteBuffer buffer = FutureIOSupport.awaitFuture(data);
-        assertDatasetEquals((int) res.getOffset(), "vecRead", buffer, res.getLength(), DATASET);
+        ByteBuffer buffer = FutureIO.awaitFuture(data);
+        assertDatasetEquals((int) res.getOffset(), "vecRead",
+                buffer, res.getLength(), originalData);
       } catch (Exception ex) {
         LOG.error("Exception while running vectored read ", ex);

Review Comment:
   remove this clause and just throw up all exceptions. look at `awaitFuture` to see what it raises



##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractSTestS3AHugeFiles.java:
##########
@@ -456,7 +456,7 @@ public void test_040_PositionedReadHugeFile() throws Throwable {
   }
 
   @Test
-  public void test_045_VectoredIOHugeFile() throws Throwable {
+  public void test_045_VectoredIoHugeFile() throws Throwable {

Review Comment:
   capital IO is OK here; your choice



##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java:
##########
@@ -1115,8 +1122,9 @@ public static void validateVectoredReadResult(List<FileRange> fileRanges, byte[]
     for (FileRange res : fileRanges) {
       CompletableFuture<ByteBuffer> data = res.getData();
       try {
-        ByteBuffer buffer = FutureIOSupport.awaitFuture(data);
-        assertDatasetEquals((int) res.getOffset(), "vecRead", buffer, res.getLength(), DATASET);
+        ByteBuffer buffer = FutureIO.awaitFuture(data);

Review Comment:
   add a timeout here for better failure messages on a timeout.



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

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


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