review comments

Author: ahmarsuhail

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/streams/AnalyticsStream.java

@@ -24,13 +24,10 @@
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Consumer;
 import java.util.function.IntFunction;
 
-import org.apache.hadoop.fs.FileRange;
-import org.apache.hadoop.fs.VectoredReadUtils;
 import software.amazon.s3.analyticsaccelerator.S3SeekableInputStreamFactory;
 import software.amazon.s3.analyticsaccelerator.S3SeekableInputStream;
 import software.amazon.s3.analyticsaccelerator.common.ObjectRange;
@@ -47,6 +44,8 @@
 import org.apache.hadoop.fs.s3a.Retries;
 import org.apache.hadoop.fs.s3a.S3AInputPolicy;
 import org.apache.hadoop.fs.s3a.S3ObjectAttributes;
+import org.apache.hadoop.fs.FileRange;
+import org.apache.hadoop.fs.VectoredReadUtils;
 
 import static org.apache.hadoop.fs.VectoredReadUtils.LOG_BYTE_BUFFER_RELEASED;
 
@@ -142,12 +141,9 @@ public int read(byte[] buf, int off, int len) throws IOException {
   }
 
   /**
-   * {@inheritDoc}
    * Pass to {@link #readVectored(List, IntFunction, Consumer)}
    * with the {@link VectoredReadUtils#LOG_BYTE_BUFFER_RELEASED} releaser.
-   * @param ranges the byte ranges to read.
-   * @param allocate the function to allocate ByteBuffer.
-   * @throws IOException IOE if any.
+   * {@inheritDoc}
    */
   @Override
   public void readVectored(List<? extends FileRange> ranges,
@@ -156,12 +152,9 @@ public void readVectored(List<? extends FileRange> ranges,
   }
 
   /**
-   * {@inheritDoc}
    * Pass to {@link #readVectored(List, IntFunction, Consumer)}
    * with the {@link VectoredReadUtils#LOG_BYTE_BUFFER_RELEASED} releaser.
-   * @param ranges the byte ranges to read.
-   * @param allocate the function to allocate ByteBuffer.
-   * @throws IOException IOE if any.
+   * {@inheritDoc}
    */
   @Override
   public void readVectored(final List<? extends FileRange> ranges,

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractAnalyticsStreamVectoredRead.java

@@ -26,14 +26,18 @@
 import org.apache.hadoop.fs.contract.ContractTestUtils;
 import org.apache.hadoop.fs.statistics.IOStatistics;
 import org.apache.hadoop.fs.statistics.StreamStatisticNames;
+import org.apache.hadoop.test.LambdaTestUtils;
 import org.apache.hadoop.test.tags.IntegrationTest;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedClass;
 import org.junit.jupiter.params.provider.MethodSource;
 
 import java.util.List;
+import java.util.function.Consumer;
 
+import static org.apache.hadoop.fs.contract.ContractTestUtils.skip;
+import static org.apache.hadoop.fs.contract.ContractTestUtils.validateVectoredReadResult;
 import static org.apache.hadoop.fs.s3a.S3ATestUtils.enableAnalyticsAccelerator;
 import static org.apache.hadoop.fs.s3a.S3ATestUtils.skipForAnyEncryptionExceptSSES3;
 import static org.apache.hadoop.fs.statistics.IOStatisticAssertions.verifyStatisticCounterValue;
@@ -73,7 +77,6 @@ protected Configuration createConfiguration() {
     // This issue is tracked in:
     // https://github.com/awslabs/analytics-accelerator-s3/issues/218
     skipForAnyEncryptionExceptSSES3(conf);
-    conf.set("fs.contract.vector-io-early-eof-check", "false");
     return conf;
   }
 
@@ -82,18 +85,31 @@ protected AbstractFSContract createContract(Configuration conf) {
     return new S3AContract(conf);
   }
 
+  /**
+   * When the offset is negative, AAL returns IllegalArgumentException, whereas the base implementation will return
+   * an EoF.
+   */
   @Override
   public void testNegativeOffsetRange()  throws Exception {
     verifyExceptionalVectoredRead(ContractTestUtils.range(-1, 50), IllegalArgumentException.class);
   }
 
+  /**
+   * Currently there is no null check on the release operation, this will be fixed in the next AAL version.
+   */
+  @Override
+  public void testNullReleaseOperation()  {
+    skip("AAL current does not do a null check on the release operation");
+  }
+
   @Test
   public void testReadVectoredWithAALStatsCollection() throws Exception {
 
     List<FileRange> fileRanges = createSampleNonOverlappingRanges();
-    try (FSDataInputStream in = openVectorFile()){
+    try (FSDataInputStream in = openVectorFile()) {
       in.readVectored(fileRanges, getAllocate());
 
+      validateVectoredReadResult(fileRanges, DATASET, 0);
       IOStatistics st = in.getIOStatistics();
 
       // Statistics such as GET requests will be added after IoStats support.