snvijaya commented on code in PR #5176:
URL: https://github.com/apache/hadoop/pull/5176#discussion_r1037798793
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsInputStream.java:
##########
@@ -495,6 +509,199 @@ public void testSuccessfulReadAhead() throws Exception {
checkEvictedStatus(inputStream, 0, true);
}
+ /**
+ * This test expects InProgressList is not purged by the inputStream close.
+ * The readBuffer will move to completedList and then finally should get
evicted.
+ */
+ @Test
+ public void testStreamPurgeDuringReadAheadCallExecuting() throws Exception {
+ AbfsClient client = getMockAbfsClient();
+ AbfsRestOperation successOp = getMockRestOp();
+
+ final AtomicInteger movedToInProgressList = new AtomicInteger(0);
+ final AtomicInteger movedToCompletedList = new AtomicInteger(0);
+ final AtomicBoolean preClosedAssertion = new AtomicBoolean(false);
+
+ Mockito.doAnswer(invocationOnMock -> {
+ movedToInProgressList.incrementAndGet();
+ while (movedToInProgressList.get() < 3 || !preClosedAssertion.get())
{
+
+ }
+ movedToCompletedList.incrementAndGet();
+ return successOp;
+ })
+ .when(client)
Review Comment:
Hi Steve, the sleep time on these mock threads are meant to hold the thread
blocked while the test goes ahead with asserts after queuing reads and asserts
after close. The sleep of 1 second (which will block the main test thread)
after queueing reads has been consistent with the timing expectations with
pre-existing tests in this class doing the same, however I agree that this test
has lot more going beyond the close which needs time synchronization, which can
make the test brittle.
Hi Pranav, The test asserts post line 566 starting from 3 sec sleep are
validations for correct movement of inprogress buffers to completed list and
their evictions, which is a functionality that this PR change does not
interfere. I would suggest that we take them out and evaluate if pre-existing
test coverage doesnt handle it already. If there are gaps, lets take it in
separate PR.
--
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-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
