You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@uniffle.apache.org by xi...@apache.org on 2023/01/16 12:47:25 UTC

[incubator-uniffle] branch master updated: [Test] Assume unknown blockID in LocalFileHandlerTestBase (#478)

This is an automated email from the ASF dual-hosted git repository.

xianjin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git


The following commit(s) were added to refs/heads/master by this push:
     new ff9c36fa [Test] Assume unknown blockID in LocalFileHandlerTestBase (#478)
ff9c36fa is described below

commit ff9c36fae13eef936ebe51f1d0ed5c7bf907ecc0
Author: Kaijie Chen <ck...@apache.org>
AuthorDate: Mon Jan 16 20:47:20 2023 +0800

    [Test] Assume unknown blockID in LocalFileHandlerTestBase (#478)
    
    ### What changes were proposed in this pull request?
    Split `LocalFileHandlerTestBase#writeTestData()` into `generateBlocks()` and `writeTestData()`.
    Pass `List<Long> blockIds` into `LocalFileHandlerTestBase#calcSegmentBytes()`.
    
    ### Why are the changes needed?
    Followup #473.
    To make the expected result corresponding to the state of `LocalFileHandlerTestBase#ATOMIC_LONG`.
    
    ### Does this PR introduce _any_ user-facing change?
    No.
    
    ### How was this patch tested?
    CI.
---
 .../storage/handler/impl/LocalFileHandlerTest.java | 29 ++++++++++++++--------
 .../handler/impl/LocalFileHandlerTestBase.java     | 26 +++++++++----------
 .../impl/LocalFileServerReadHandlerTest.java       | 16 ++++++------
 3 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/storage/src/test/java/org/apache/uniffle/storage/handler/impl/LocalFileHandlerTest.java b/storage/src/test/java/org/apache/uniffle/storage/handler/impl/LocalFileHandlerTest.java
index a0c0043f..a5c99674 100644
--- a/storage/src/test/java/org/apache/uniffle/storage/handler/impl/LocalFileHandlerTest.java
+++ b/storage/src/test/java/org/apache/uniffle/storage/handler/impl/LocalFileHandlerTest.java
@@ -63,15 +63,23 @@ public class LocalFileHandlerTest {
     final Set<Long> expectedBlockIds1 = Sets.newHashSet();
     final Set<Long> expectedBlockIds2 = Sets.newHashSet();
 
-    LocalFileHandlerTestBase.writeTestData(writeHandler1, 1, 32, expectedData, expectedBlockIds1);
-    LocalFileHandlerTestBase.writeTestData(writeHandler1, 2, 32, expectedData, expectedBlockIds1);
-    LocalFileHandlerTestBase.writeTestData(writeHandler1, 3, 32, expectedData, expectedBlockIds1);
-    LocalFileHandlerTestBase.writeTestData(writeHandler1, 4, 32, expectedData, expectedBlockIds1);
-
-    LocalFileHandlerTestBase.writeTestData(writeHandler2, 3, 32, expectedData, expectedBlockIds2);
-    LocalFileHandlerTestBase.writeTestData(writeHandler2, 3, 32, expectedData, expectedBlockIds2);
-    LocalFileHandlerTestBase.writeTestData(writeHandler2, 2, 32, expectedData, expectedBlockIds2);
-    LocalFileHandlerTestBase.writeTestData(writeHandler2, 1, 32, expectedData, expectedBlockIds2);
+    LocalFileHandlerTestBase.writeTestData(LocalFileHandlerTestBase.generateBlocks(1, 32),
+        writeHandler1, expectedData, expectedBlockIds1);
+    LocalFileHandlerTestBase.writeTestData(LocalFileHandlerTestBase.generateBlocks(2, 32),
+        writeHandler1, expectedData, expectedBlockIds1);
+    LocalFileHandlerTestBase.writeTestData(LocalFileHandlerTestBase.generateBlocks(3, 32),
+        writeHandler1, expectedData, expectedBlockIds1);
+    LocalFileHandlerTestBase.writeTestData(LocalFileHandlerTestBase.generateBlocks(4, 32),
+        writeHandler1, expectedData, expectedBlockIds1);
+
+    LocalFileHandlerTestBase.writeTestData(LocalFileHandlerTestBase.generateBlocks(3, 32),
+        writeHandler2, expectedData, expectedBlockIds2);
+    LocalFileHandlerTestBase.writeTestData(LocalFileHandlerTestBase.generateBlocks(3, 32),
+        writeHandler2, expectedData, expectedBlockIds2);
+    LocalFileHandlerTestBase.writeTestData(LocalFileHandlerTestBase.generateBlocks(2, 32),
+        writeHandler2, expectedData, expectedBlockIds2);
+    LocalFileHandlerTestBase.writeTestData(LocalFileHandlerTestBase.generateBlocks(1, 32),
+        writeHandler2, expectedData, expectedBlockIds2);
 
     RssBaseConf conf = new RssBaseConf();
     conf.setString("rss.storage.basePath", dataDir1.getAbsolutePath() + "," + dataDir2.getAbsolutePath());
@@ -84,7 +92,8 @@ public class LocalFileHandlerTest {
     LocalFileHandlerTestBase.validateResult(readHandler2, expectedBlockIds2, expectedData);
 
     // after first read, write more data
-    LocalFileHandlerTestBase.writeTestData(writeHandler1, 1, 32, expectedData, expectedBlockIds1);
+    LocalFileHandlerTestBase.writeTestData(LocalFileHandlerTestBase.generateBlocks(1, 32),
+        writeHandler1, expectedData, expectedBlockIds1);
     // new data should be read
     LocalFileHandlerTestBase.validateResult(readHandler1, expectedBlockIds1, expectedData);
 
diff --git a/storage/src/test/java/org/apache/uniffle/storage/handler/impl/LocalFileHandlerTestBase.java b/storage/src/test/java/org/apache/uniffle/storage/handler/impl/LocalFileHandlerTestBase.java
index c1458f8f..89eb24d5 100644
--- a/storage/src/test/java/org/apache/uniffle/storage/handler/impl/LocalFileHandlerTestBase.java
+++ b/storage/src/test/java/org/apache/uniffle/storage/handler/impl/LocalFileHandlerTestBase.java
@@ -44,15 +44,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 public class LocalFileHandlerTestBase {
   private static AtomicLong ATOMIC_LONG = new AtomicLong(0L);
 
-  public static void reset() {
-    ATOMIC_LONG = new AtomicLong(0L);
-  }
-
-  public static void writeTestData(
-      ShuffleWriteHandler writeHandler,
-      int num, int length,
-      Map<Long, byte[]> expectedData,
-      Set<Long> expectedBlockIds) throws Exception {
+  public static List<ShufflePartitionedBlock> generateBlocks(int num, int length) {
     List<ShufflePartitionedBlock> blocks = Lists.newArrayList();
     for (int i = 0; i < num; i++) {
       byte[] buf = new byte[length];
@@ -60,10 +52,15 @@ public class LocalFileHandlerTestBase {
       long blockId = ATOMIC_LONG.incrementAndGet();
       blocks.add(new ShufflePartitionedBlock(length, length, ChecksumUtils.getCrc32(buf), blockId, 100,
           buf));
-      expectedData.put(blockId, buf);
-      expectedBlockIds.add(blockId);
     }
-    writeHandler.write(blocks);
+    return blocks;
+  }
+
+  public static void writeTestData(List<ShufflePartitionedBlock> blocks, ShuffleWriteHandler handler,
+      Map<Long, byte[]> expectedData, Set<Long> expectedBlockIds) throws Exception {
+    handler.write(blocks);
+    blocks.forEach(block -> expectedBlockIds.add(block.getBlockId()));
+    blocks.forEach(block -> expectedData.put(block.getBlockId(), block.getData()));
   }
 
   public static void validateResult(ServerReadHandler readHandler, Set<Long> expectedBlockIds,
@@ -134,11 +131,12 @@ public class LocalFileHandlerTestBase {
     byteBuffer.putLong(segment.getTaskAttemptId());
   }
 
-  public static List<byte[]> calcSegmentBytes(Map<Long, byte[]> blockIdToData, int bytesPerSegment, int blockNum) {
+  public static List<byte[]> calcSegmentBytes(Map<Long, byte[]> blockIdToData,
+      int bytesPerSegment, List<Long> blockIds) {
     List<byte[]> res = Lists.newArrayList();
     int curSize = 0;
     ByteBuffer byteBuffer = ByteBuffer.allocate(2 * bytesPerSegment);
-    for (long i = 1; i <= blockNum; i++) {
+    for (long i : blockIds) {
       byte[] data = blockIdToData.get(i);
       byteBuffer.put(data, 0, data.length);
       curSize += data.length;
diff --git a/storage/src/test/java/org/apache/uniffle/storage/handler/impl/LocalFileServerReadHandlerTest.java b/storage/src/test/java/org/apache/uniffle/storage/handler/impl/LocalFileServerReadHandlerTest.java
index f247c2d4..a8d4ec25 100644
--- a/storage/src/test/java/org/apache/uniffle/storage/handler/impl/LocalFileServerReadHandlerTest.java
+++ b/storage/src/test/java/org/apache/uniffle/storage/handler/impl/LocalFileServerReadHandlerTest.java
@@ -21,6 +21,7 @@ import java.nio.ByteBuffer;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 import com.google.common.collect.Maps;
 import org.junit.jupiter.api.Test;
@@ -43,8 +44,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 public class LocalFileServerReadHandlerTest {
   @Test
   public void testDataInconsistent() throws Exception {
-    LocalFileHandlerTestBase.reset();
-
     Map<Long, byte[]> expectedData = Maps.newHashMap();
     int expectTotalBlockNum = 4;
     int blockSize = 7;
@@ -53,7 +52,8 @@ public class LocalFileServerReadHandlerTest {
     Roaring64NavigableMap expectBlockIds = Roaring64NavigableMap.bitmapOf();
 
     // We simulate the generation of 4 block index files and 3 block data files to test LocalFileClientReadHandler
-    LocalFileHandlerTestBase.writeTestData(shuffleBlocks -> {
+    List<ShufflePartitionedBlock> blocks = LocalFileHandlerTestBase.generateBlocks(expectTotalBlockNum, blockSize);
+    LocalFileHandlerTestBase.writeTestData(blocks, shuffleBlocks -> {
       int offset = 0;
       for (ShufflePartitionedBlock block : shuffleBlocks) {
         FileBasedShuffleSegment segment = new FileBasedShuffleSegment(
@@ -62,9 +62,9 @@ public class LocalFileServerReadHandlerTest {
         offset += block.getLength();
         LocalFileHandlerTestBase.writeIndex(byteBuffer, segment);
       }
-    }, expectTotalBlockNum, blockSize,
-        expectedData, new HashSet<>());
-    expectedData.forEach((id, block) -> expectBlockIds.addLong(id));
+    }, expectedData, new HashSet<>());
+
+    blocks.forEach(block -> expectBlockIds.addLong(block.getBlockId()));
 
     String appId = "app1";
     int shuffleId = 1;
@@ -79,8 +79,10 @@ public class LocalFileServerReadHandlerTest {
 
     int readBufferSize = 13;
     int bytesPerSegment = ((readBufferSize / blockSize) + 1) * blockSize;
+    List<Long> actualWriteBlockIds = blocks.stream().map(ShufflePartitionedBlock::getBlockId)
+        .limit(actualWriteDataBlock).collect(Collectors.toList());
     List<byte[]> segments = LocalFileHandlerTestBase.calcSegmentBytes(expectedData,
-        bytesPerSegment, actualWriteDataBlock);
+        bytesPerSegment, actualWriteBlockIds);
 
     // first segment include 2 blocks
     ArgumentMatcher<RssGetShuffleDataRequest> segment1Match =