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


##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3APrefetchingInputStream.java:
##########
@@ -240,4 +242,56 @@ public void testRandomReadSmallFile() throws Throwable {
     }
   }
 
+  @Test
+  public void testStatusProbesAfterClosingStream() throws Throwable {
+    describe("When the underlying input stream is closed, the prefetch input 
stream"
+        + " should still support some status probes");
+
+    byte[] data = ContractTestUtils.dataset(SMALL_FILE_SIZE, 'a', 26);
+    Path smallFile = path("testStatusProbesAfterClosingStream");

Review Comment:
   you can just use `path()` and one is built up from the method name. this is 
better as it avoids cut and paste bugs *and* if a build is parameterized, the 
parameterized string is used in the path (though tests fail if that string 
isn't a valid path any more)



##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3APrefetchingInputStream.java:
##########
@@ -240,4 +242,56 @@ public void testRandomReadSmallFile() throws Throwable {
     }
   }
 
+  @Test
+  public void testStatusProbesAfterClosingStream() throws Throwable {
+    describe("When the underlying input stream is closed, the prefetch input 
stream"
+        + " should still support some status probes");
+
+    byte[] data = ContractTestUtils.dataset(SMALL_FILE_SIZE, 'a', 26);
+    Path smallFile = path("testStatusProbesAfterClosingStream");
+    ContractTestUtils.writeDataset(getFileSystem(), smallFile, data, 
data.length, 16, true);
+
+    FSDataInputStream in = getFileSystem().open(smallFile);
+
+    byte[] buffer = new byte[SMALL_FILE_SIZE];
+    in.read(buffer, 0, S_1K * 4);
+    in.seek(S_1K * 12);
+    in.read(buffer, 0, S_1K * 4);
+
+    long pos = in.getPos();
+    IOStatistics ioStats = in.getIOStatistics();
+    S3AInputStreamStatistics inputStreamStatistics =
+        ((S3APrefetchingInputStream) 
(in.getWrappedStream())).getS3AStreamStatistics();
+
+    assertNotNull("Prefetching input IO stats should not be null", ioStats);
+    assertNotNull("Prefetching input stream stats should not be null", 
inputStreamStatistics);
+    assertNotEquals("Position retrieved from prefetching input stream should 
be greater than 0", 0,
+        pos);
+
+    in.close();
+
+    // status probes after closing the input stream
+    long newPos = in.getPos();
+    IOStatistics newIoStats = in.getIOStatistics();
+    S3AInputStreamStatistics newInputStreamStatistics =
+        ((S3APrefetchingInputStream) 
(in.getWrappedStream())).getS3AStreamStatistics();
+
+    assertNotNull("Prefetching input IO stats should not be null", newIoStats);
+    assertNotNull("Prefetching input stream stats should not be null", 
newInputStreamStatistics);
+    assertNotEquals("Position retrieved from prefetching input stream should 
be greater than 0", 0,
+        newPos);
+
+    // compare status probes after closing of the stream with status probes 
done before
+    // closing the stream
+    assertEquals("Position retrieved through stream before and after closing 
should match", pos,
+        newPos);
+    assertEquals("IO stats retrieved through stream before and after closing 
should match", ioStats,
+        newIoStats);
+    assertEquals("Stream stats retrieved through stream before and after 
closing should match",
+        inputStreamStatistics, newInputStreamStatistics);
+
+    assertFalse("Not supported with prefetch", in.seekToNewSource(10));

Review Comment:
   nit: can you have the error message include "seekToNewSource()". 



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