Copilot commented on code in PR #2696:
URL: https://github.com/apache/uniffle/pull/2696#discussion_r2605601543
##########
storage/src/test/java/org/apache/uniffle/storage/handler/impl/PrefetchableClientReadHandlerTest.java:
##########
@@ -59,19 +80,39 @@ protected ShuffleDataResult doReadShuffleData() {
// ignore
}
}
- if (readNum.get() > 0) {
- readNum.decrementAndGet();
- return new ShuffleDataResult();
+ if (maxReadLoopNum.get() > 0) {
+ maxReadLoopNum.decrementAndGet();
+ List<BufferSegment> segments = new ArrayList<>();
+ segments.add(new BufferSegment(1, 1, 1, 1, 1, 1));
+ return new ShuffleDataResult(new byte[10], segments);
}
return null;
}
}
@Test
- public void test_with_prefetch() {
+ public void testWithPrefetchWithEmptyResult() {
+ AtomicInteger maxReadLoopNum = new AtomicInteger(10);
PrefetchableClientReadHandler handler =
new MockedHandler(
- Optional.of(new PrefetchableClientReadHandler.PrefetchOption(4,
10)), 10, false, false);
+ Optional.of(new PrefetchableClientReadHandler.PrefetchOption(4,
10)),
+ maxReadLoopNum,
+ false,
+ false,
+ true);
+ assertTrue(handler.readShuffleData().isEmpty());
Review Comment:
The test verifies that the first read returns an empty result, but doesn't
verify that subsequent reads also handle empty results correctly. Consider
adding assertions to verify the behavior after multiple reads when the handler
consistently returns empty results.
##########
storage/src/test/java/org/apache/uniffle/storage/handler/impl/PrefetchableClientReadHandlerTest.java:
##########
@@ -59,19 +80,39 @@ protected ShuffleDataResult doReadShuffleData() {
// ignore
}
}
- if (readNum.get() > 0) {
- readNum.decrementAndGet();
- return new ShuffleDataResult();
+ if (maxReadLoopNum.get() > 0) {
+ maxReadLoopNum.decrementAndGet();
+ List<BufferSegment> segments = new ArrayList<>();
+ segments.add(new BufferSegment(1, 1, 1, 1, 1, 1));
+ return new ShuffleDataResult(new byte[10], segments);
}
return null;
}
}
@Test
- public void test_with_prefetch() {
+ public void testWithPrefetchWithEmptyResult() {
+ AtomicInteger maxReadLoopNum = new AtomicInteger(10);
PrefetchableClientReadHandler handler =
new MockedHandler(
- Optional.of(new PrefetchableClientReadHandler.PrefetchOption(4,
10)), 10, false, false);
+ Optional.of(new PrefetchableClientReadHandler.PrefetchOption(4,
10)),
+ maxReadLoopNum,
+ false,
+ false,
+ true);
+ assertTrue(handler.readShuffleData().isEmpty());
+ Awaitility.await().until(() -> handler.isFinished());
+ assertEquals(9, maxReadLoopNum.get());
Review Comment:
This assertion verifies that doReadShuffleData() was called once, but
doesn't verify that the prefetch thread also stopped correctly. Consider adding
an assertion to verify that no additional prefetch operations were queued after
the empty result was encountered.
--
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]
