errose28 commented on code in PR #7293:
URL: https://github.com/apache/ozone/pull/7293#discussion_r1833411342
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java:
##########
@@ -131,21 +141,221 @@ public static ContainerMerkleTree
buildTestTree(ConfigurationSource conf) {
final long blockID1 = 1;
final long blockID2 = 2;
final long blockID3 = 3;
+ final long blockID4 = 4;
+ final long blockID5 = 5;
ContainerProtos.ChunkInfo b1c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{1, 2, 3}));
ContainerProtos.ChunkInfo b1c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{4, 5, 6}));
- ContainerProtos.ChunkInfo b2c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{7, 8, 9}));
- ContainerProtos.ChunkInfo b2c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{12, 11, 10}));
- ContainerProtos.ChunkInfo b3c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{13, 14, 15}));
- ContainerProtos.ChunkInfo b3c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{16, 17, 18}));
+ ContainerProtos.ChunkInfo b1c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{7, 8, 9}));
+ ContainerProtos.ChunkInfo b1c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{12, 11, 10}));
+ ContainerProtos.ChunkInfo b2c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{13, 14, 15}));
+ ContainerProtos.ChunkInfo b2c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{16, 17, 18}));
+ ContainerProtos.ChunkInfo b2c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{19, 20, 21}));
+ ContainerProtos.ChunkInfo b2c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{22, 23, 24}));
+ ContainerProtos.ChunkInfo b3c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{25, 26, 27}));
+ ContainerProtos.ChunkInfo b3c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{28, 29, 30}));
+ ContainerProtos.ChunkInfo b3c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{31, 32, 33}));
+ ContainerProtos.ChunkInfo b3c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{34, 35, 36}));
+ ContainerProtos.ChunkInfo b4c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{37, 38, 29}));
+ ContainerProtos.ChunkInfo b4c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{40, 41, 42}));
+ ContainerProtos.ChunkInfo b4c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{43, 44, 45}));
+ ContainerProtos.ChunkInfo b4c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{46, 47, 48}));
+ ContainerProtos.ChunkInfo b5c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{49, 50, 51}));
+ ContainerProtos.ChunkInfo b5c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{52, 53, 54}));
+ ContainerProtos.ChunkInfo b5c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{55, 56, 57}));
+ ContainerProtos.ChunkInfo b5c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{58, 59, 60}));
ContainerMerkleTree tree = new ContainerMerkleTree();
- tree.addChunks(blockID1, Arrays.asList(b1c1, b1c2));
- tree.addChunks(blockID2, Arrays.asList(b2c1, b2c2));
- tree.addChunks(blockID3, Arrays.asList(b3c1, b3c2));
+ tree.addChunks(blockID1, Arrays.asList(b1c1, b1c2, b1c3, b1c4));
+ tree.addChunks(blockID2, Arrays.asList(b2c1, b2c2, b2c3, b2c4));
+ tree.addChunks(blockID3, Arrays.asList(b3c1, b3c2, b3c3, b3c4));
+ tree.addChunks(blockID4, Arrays.asList(b4c1, b4c2, b4c3, b4c4));
+ tree.addChunks(blockID5, Arrays.asList(b5c1, b5c2, b5c3, b5c4));
return tree;
}
+ public static Pair<ContainerMerkleTree,
ContainerChecksumTreeManager.ContainerDiff> buildTestTreeWithMismatches(
+ ConfigurationSource conf, int numMissingBlocks, int numMissingChunks,
int numCorruptChunks) {
+
+ ContainerMerkleTree tree = buildTestTree(conf);
+ ContainerChecksumTreeManager.ContainerDiff diff = new
ContainerChecksumTreeManager.ContainerDiff();
+ Random random = new Random();
+
+ List<Long> blockIds = new ArrayList<>(Arrays.asList(1L, 2L, 3L, 4L, 5L));
+
+ introduceMissingBlocks(tree, diff, blockIds, numMissingBlocks, random);
+ introduceMissingChunks(tree, diff, blockIds, numMissingChunks, random);
+ introduceCorruptChunks(tree, diff, blockIds, numCorruptChunks, random,
conf);
+
+ return Pair.of(tree, diff);
+ }
+
+ private static void introduceMissingBlocks(ContainerMerkleTree tree,
+
ContainerChecksumTreeManager.ContainerDiff diff,
+ List<Long> blockIds,
+ int numMissingBlocks,
+ Random random) {
+ for (int i = 0; i < numMissingBlocks && !blockIds.isEmpty(); i++) {
+ int index = random.nextInt(blockIds.size());
+ long blockId = blockIds.remove(index);
+ ContainerMerkleTree.BlockMerkleTree blockTree = tree.remove(blockId);
+ diff.addMissingBlock(blockTree.toProto());
+ }
+ }
+
+ private static void introduceMissingChunks(ContainerMerkleTree tree,
+
ContainerChecksumTreeManager.ContainerDiff diff,
+ List<Long> blockIds,
+ int numMissingChunks,
+ Random random) {
+ for (int i = 0; i < numMissingChunks && !blockIds.isEmpty(); i++) {
+ long blockId = blockIds.get(random.nextInt(blockIds.size()));
+ ContainerMerkleTree.BlockMerkleTree blockTree = tree.get(blockId);
+ List<Long> chunkOffsets = getChunkOffsets(blockTree);
+
+ if (!chunkOffsets.isEmpty()) {
+ long offset = chunkOffsets.remove(random.nextInt(chunkOffsets.size()));
+ ContainerMerkleTree.ChunkMerkleTree chunkTree =
blockTree.removeChunk(offset);
+ diff.addMissingChunk(blockId, chunkTree.toProto());
+ }
+ }
+ }
+
+ private static void introduceCorruptChunks(ContainerMerkleTree tree,
+
ContainerChecksumTreeManager.ContainerDiff diff,
+ List<Long> blockIds,
+ int numCorruptChunks,
+ Random random,
+ ConfigurationSource conf) {
+ // Create a map to keep track of corrupted chunks per block
+ Map<Long, Set<Long>> corruptedChunksPerBlock = new HashMap<>();
+
+ int corruptionsIntroduced = 0;
+ while (corruptionsIntroduced < numCorruptChunks && !blockIds.isEmpty()) {
+ // Randomly select a block
+ int blockIndex = random.nextInt(blockIds.size());
+ long blockId = blockIds.get(blockIndex);
+ ContainerMerkleTree.BlockMerkleTree blockTree = tree.get(blockId);
+
+ // Get available chunk offsets for this block
+ List<Long> availableChunkOffsets = getChunkOffsets(blockTree);
+
+ // Remove already corrupted chunks for this block
+
availableChunkOffsets.removeAll(corruptedChunksPerBlock.getOrDefault(blockId,
new HashSet<>()));
+
+ if (!availableChunkOffsets.isEmpty()) {
+ // Randomly select an available chunk offset
+ int chunkIndex = random.nextInt(availableChunkOffsets.size());
+ long offset = availableChunkOffsets.get(chunkIndex);
+
+ // Remove the original chunk
+ ContainerMerkleTree.ChunkMerkleTree chunkMerkleTree =
blockTree.removeChunk(offset);
+
+ // Create and add corrupt chunk
+ ContainerProtos.ChunkInfo corruptChunk = buildChunk(conf, (int)
offset, ByteBuffer.wrap(new byte[]{5, 10, 15}));
+ tree.addChunk(blockId, corruptChunk);
+ blockTree.setHealthy(offset, false);
+ diff.addCorruptChunks(blockId, chunkMerkleTree.toProto());
+
+ // Mark this chunk as corrupted for this block
+ corruptedChunksPerBlock.computeIfAbsent(blockId, k -> new
HashSet<>()).add(offset);
+
+ corruptionsIntroduced++;
+ } else {
+ // If no available chunks in this block, remove it from consideration
+ blockIds.remove(blockIndex);
+ }
+ }
+ }
+
+ private static List<Long>
getChunkOffsets(ContainerMerkleTree.BlockMerkleTree blockTree) {
+ return blockTree.toProto().getChunkMerkleTreeList().stream()
+ .map(ContainerProtos.ChunkMerkleTree::getOffset)
+ .collect(Collectors.toList());
+ }
+
+ public static void
assertContainerDiffMatch(ContainerChecksumTreeManager.ContainerDiff
expectedDiff,
+ ContainerChecksumTreeManager.ContainerDiff actualDiff) {
+ assertNotNull(expectedDiff, "Expected diff is null");
+ assertNotNull(actualDiff, "Actual diff is null");
+ assertEquals(expectedDiff.getMissingBlocks().size(),
actualDiff.getMissingBlocks().size(),
+ "Mismatch in number of missing blocks");
+ assertEquals(expectedDiff.getMissingChunks().size(),
actualDiff.getMissingChunks().size(),
+ "Mismatch in number of missing chunks");
+ assertEquals(expectedDiff.getCorruptChunks().size(),
actualDiff.getCorruptChunks().size(),
+ "Mismatch in number of corrupt blocks");
+
+ List<ContainerProtos.BlockMerkleTree> expectedMissingBlocks =
expectedDiff.getMissingBlocks().stream().sorted(
+
Comparator.comparing(ContainerProtos.BlockMerkleTree::getBlockID)).collect(Collectors.toList());
+ List<ContainerProtos.BlockMerkleTree> actualMissingBlocks =
expectedDiff.getMissingBlocks().stream().sorted(
+
Comparator.comparing(ContainerProtos.BlockMerkleTree::getBlockID)).collect(Collectors.toList());
+ for (int i = 0; i < expectedMissingBlocks.size(); i++) {
+ ContainerProtos.BlockMerkleTree expectedBlockMerkleTree =
expectedMissingBlocks.get(i);
+ ContainerProtos.BlockMerkleTree actualBlockMerkleTree =
actualMissingBlocks.get(i);
+ assertEquals(expectedBlockMerkleTree.getBlockID(),
actualBlockMerkleTree.getBlockID());
+ assertEquals(expectedBlockMerkleTree.getChunkMerkleTreeCount(),
+ actualBlockMerkleTree.getChunkMerkleTreeCount());
+ assertEquals(expectedBlockMerkleTree.getBlockChecksum(),
actualBlockMerkleTree.getBlockChecksum());
+
+ for (int j = 0; j < expectedBlockMerkleTree.getChunkMerkleTreeCount();
j++) {
+ ContainerProtos.ChunkMerkleTree expectedChunk =
expectedBlockMerkleTree.getChunkMerkleTree(j);
+ ContainerProtos.ChunkMerkleTree actualChunk =
expectedBlockMerkleTree.getChunkMerkleTree(j);
+ assertEquals(expectedChunk.getOffset(), actualChunk.getOffset(),
"Mismatch in chunk offset");
+ assertEquals(expectedChunk.getChunkChecksum(),
actualChunk.getChunkChecksum(), "Mismatch in chunk checksum");
Review Comment:
Can we move the chunk merkle tree equality check into a helper method and
call it a needed for the three main checks in this method?
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java:
##########
@@ -299,6 +319,64 @@ public void testEmptyFile() throws Exception {
assertEquals(CONTAINER_ID, info.getContainerID());
}
+ @Test
+ public void testContainerWithNoDiff() throws IOException {
+ ContainerMerkleTree ourMerkleTree = buildTestTree(config);
+ ContainerMerkleTree peerMerkleTree = buildTestTree(config);
+ checksumManager.writeContainerDataTree(container, ourMerkleTree);
+ ContainerChecksumTreeManager containerChecksumTreeManager = new
ContainerChecksumTreeManager(
Review Comment:
We can just use the `checksumManager` field without this new object right?
Same for the other tests.
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java:
##########
@@ -59,6 +66,19 @@ class TestContainerChecksumTreeManager {
private ContainerMerkleTreeMetrics metrics;
private ConfigurationSource config;
+ public static Stream<Arguments> getParameters() {
Review Comment:
Let's give this a more informative name indicating what the parameters mean,
like `getFaultCombinations` or something like that. We can also add a comment
here explaining what the number at each position means since its a little
confusing without the context of the test method.
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java:
##########
@@ -131,21 +141,221 @@ public static ContainerMerkleTree
buildTestTree(ConfigurationSource conf) {
final long blockID1 = 1;
final long blockID2 = 2;
final long blockID3 = 3;
+ final long blockID4 = 4;
+ final long blockID5 = 5;
ContainerProtos.ChunkInfo b1c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{1, 2, 3}));
ContainerProtos.ChunkInfo b1c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{4, 5, 6}));
- ContainerProtos.ChunkInfo b2c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{7, 8, 9}));
- ContainerProtos.ChunkInfo b2c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{12, 11, 10}));
- ContainerProtos.ChunkInfo b3c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{13, 14, 15}));
- ContainerProtos.ChunkInfo b3c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{16, 17, 18}));
+ ContainerProtos.ChunkInfo b1c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{7, 8, 9}));
+ ContainerProtos.ChunkInfo b1c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{12, 11, 10}));
+ ContainerProtos.ChunkInfo b2c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{13, 14, 15}));
+ ContainerProtos.ChunkInfo b2c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{16, 17, 18}));
+ ContainerProtos.ChunkInfo b2c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{19, 20, 21}));
+ ContainerProtos.ChunkInfo b2c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{22, 23, 24}));
+ ContainerProtos.ChunkInfo b3c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{25, 26, 27}));
+ ContainerProtos.ChunkInfo b3c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{28, 29, 30}));
+ ContainerProtos.ChunkInfo b3c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{31, 32, 33}));
+ ContainerProtos.ChunkInfo b3c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{34, 35, 36}));
+ ContainerProtos.ChunkInfo b4c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{37, 38, 29}));
+ ContainerProtos.ChunkInfo b4c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{40, 41, 42}));
+ ContainerProtos.ChunkInfo b4c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{43, 44, 45}));
+ ContainerProtos.ChunkInfo b4c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{46, 47, 48}));
+ ContainerProtos.ChunkInfo b5c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{49, 50, 51}));
+ ContainerProtos.ChunkInfo b5c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{52, 53, 54}));
+ ContainerProtos.ChunkInfo b5c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{55, 56, 57}));
+ ContainerProtos.ChunkInfo b5c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{58, 59, 60}));
ContainerMerkleTree tree = new ContainerMerkleTree();
- tree.addChunks(blockID1, Arrays.asList(b1c1, b1c2));
- tree.addChunks(blockID2, Arrays.asList(b2c1, b2c2));
- tree.addChunks(blockID3, Arrays.asList(b3c1, b3c2));
+ tree.addChunks(blockID1, Arrays.asList(b1c1, b1c2, b1c3, b1c4));
+ tree.addChunks(blockID2, Arrays.asList(b2c1, b2c2, b2c3, b2c4));
+ tree.addChunks(blockID3, Arrays.asList(b3c1, b3c2, b3c3, b3c4));
+ tree.addChunks(blockID4, Arrays.asList(b4c1, b4c2, b4c3, b4c4));
+ tree.addChunks(blockID5, Arrays.asList(b5c1, b5c2, b5c3, b5c4));
Review Comment:
Now that we are building a bigger tree can we refactor this as a loop?
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java:
##########
@@ -299,6 +319,64 @@ public void testEmptyFile() throws Exception {
assertEquals(CONTAINER_ID, info.getContainerID());
}
+ @Test
+ public void testContainerWithNoDiff() throws IOException {
+ ContainerMerkleTree ourMerkleTree = buildTestTree(config);
+ ContainerMerkleTree peerMerkleTree = buildTestTree(config);
+ checksumManager.writeContainerDataTree(container, ourMerkleTree);
+ ContainerChecksumTreeManager containerChecksumTreeManager = new
ContainerChecksumTreeManager(
+ new OzoneConfiguration());
+ ContainerProtos.ContainerChecksumInfo peerChecksumInfo =
ContainerProtos.ContainerChecksumInfo.newBuilder()
+ .setContainerID(container.getContainerID())
+ .setContainerMerkleTree(peerMerkleTree.toProto()).build();
+ ContainerChecksumTreeManager.ContainerDiff diff =
+ containerChecksumTreeManager.diff(container, peerChecksumInfo);
+ assertTrue(diff.isHealthy());
+ }
+
+ @ParameterizedTest
+ @MethodSource("getParameters")
+ public void testContainerDiffWithMismatches(int numMissingBlock, int
numMissingChunk,
+ int numCorruptChunk) throws
IOException {
+ Pair<ContainerMerkleTree, ContainerChecksumTreeManager.ContainerDiff>
buildResult =
+ buildTestTreeWithMismatches(config, numMissingBlock, numMissingChunk,
numCorruptChunk);
+ ContainerChecksumTreeManager.ContainerDiff expectedDiff =
buildResult.getRight();
+ ContainerMerkleTree ourMerkleTree = buildResult.getLeft();
+ ContainerMerkleTree peerMerkleTree = buildTestTree(config);
+ checksumManager.writeContainerDataTree(container, ourMerkleTree);
+ ContainerChecksumTreeManager containerChecksumTreeManager = new
ContainerChecksumTreeManager(
+ new OzoneConfiguration());
+ ContainerProtos.ContainerChecksumInfo peerChecksumInfo =
ContainerProtos.ContainerChecksumInfo.newBuilder()
+ .setContainerID(container.getContainerID())
+ .setContainerMerkleTree(peerMerkleTree.toProto()).build();
+ ContainerChecksumTreeManager.ContainerDiff diff =
+ containerChecksumTreeManager.diff(container, peerChecksumInfo);
+ assertContainerDiffMatch(expectedDiff, diff);
+ }
+
+ /**
+ * Test if a peer which has missing blocks and chunks affects our container
diff.
+ * Only if our merkle tree has missing entries from the peer we need to add
it the Container Diff.
+ */
+ @ParameterizedTest
+ @MethodSource("getParameters")
+ public void testPeerWithMissingBlockAndMissingChunks(int numMissingBlock,
int numMissingChunk,
Review Comment:
I think we can clarify in the name and comments for these tests that this
one is testing that we do not do repairs for a peer's errors, and the previous
one is testing that we do repairs when we have errors. Both tests are
important, but I had to look closely to figure out the difference.
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java:
##########
@@ -59,6 +66,19 @@ class TestContainerChecksumTreeManager {
private ContainerMerkleTreeMetrics metrics;
private ConfigurationSource config;
+ public static Stream<Arguments> getParameters() {
+ return Stream.of(
+ Arguments.of(1, 2, 3),
+ Arguments.of(2, 3, 1),
+ Arguments.of(3, 1, 2),
+ Arguments.of(2, 2, 3),
+ Arguments.of(3, 2, 2),
+ Arguments.of(3, 3, 3),
+ Arguments.of(2, 1, 4),
+ Arguments.of(2, 3, 4),
+ Arguments.of(1, 2, 4));
Review Comment:
Broadly speaking the three cases to test are zero, one, or many. Zero and
one are useful to isolate problems before proceeding to more complex tests, so
we should definitely have these in the matrix:
```
(0, 0, 0)
(0, 0, 1)
(0, 1, 0)
(1, 0, 0)
```
Next we want to test combinations of faults. We want to make sure that one
type of detection does not depend on another, so the matrix might have these
entries as well:
```
(3, 3, 3)
(3, 3, 0)
(3, 0, 3)
(0, 3, 3)
```
I think this will provide decent coverage, but it might need to be updated
when we add deleted block testing.
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java:
##########
@@ -131,21 +141,221 @@ public static ContainerMerkleTree
buildTestTree(ConfigurationSource conf) {
final long blockID1 = 1;
final long blockID2 = 2;
final long blockID3 = 3;
+ final long blockID4 = 4;
+ final long blockID5 = 5;
ContainerProtos.ChunkInfo b1c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{1, 2, 3}));
ContainerProtos.ChunkInfo b1c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{4, 5, 6}));
- ContainerProtos.ChunkInfo b2c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{7, 8, 9}));
- ContainerProtos.ChunkInfo b2c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{12, 11, 10}));
- ContainerProtos.ChunkInfo b3c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{13, 14, 15}));
- ContainerProtos.ChunkInfo b3c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{16, 17, 18}));
+ ContainerProtos.ChunkInfo b1c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{7, 8, 9}));
+ ContainerProtos.ChunkInfo b1c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{12, 11, 10}));
+ ContainerProtos.ChunkInfo b2c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{13, 14, 15}));
+ ContainerProtos.ChunkInfo b2c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{16, 17, 18}));
+ ContainerProtos.ChunkInfo b2c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{19, 20, 21}));
+ ContainerProtos.ChunkInfo b2c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{22, 23, 24}));
+ ContainerProtos.ChunkInfo b3c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{25, 26, 27}));
+ ContainerProtos.ChunkInfo b3c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{28, 29, 30}));
+ ContainerProtos.ChunkInfo b3c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{31, 32, 33}));
+ ContainerProtos.ChunkInfo b3c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{34, 35, 36}));
+ ContainerProtos.ChunkInfo b4c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{37, 38, 29}));
+ ContainerProtos.ChunkInfo b4c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{40, 41, 42}));
+ ContainerProtos.ChunkInfo b4c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{43, 44, 45}));
+ ContainerProtos.ChunkInfo b4c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{46, 47, 48}));
+ ContainerProtos.ChunkInfo b5c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{49, 50, 51}));
+ ContainerProtos.ChunkInfo b5c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{52, 53, 54}));
+ ContainerProtos.ChunkInfo b5c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{55, 56, 57}));
+ ContainerProtos.ChunkInfo b5c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{58, 59, 60}));
ContainerMerkleTree tree = new ContainerMerkleTree();
- tree.addChunks(blockID1, Arrays.asList(b1c1, b1c2));
- tree.addChunks(blockID2, Arrays.asList(b2c1, b2c2));
- tree.addChunks(blockID3, Arrays.asList(b3c1, b3c2));
+ tree.addChunks(blockID1, Arrays.asList(b1c1, b1c2, b1c3, b1c4));
+ tree.addChunks(blockID2, Arrays.asList(b2c1, b2c2, b2c3, b2c4));
+ tree.addChunks(blockID3, Arrays.asList(b3c1, b3c2, b3c3, b3c4));
+ tree.addChunks(blockID4, Arrays.asList(b4c1, b4c2, b4c3, b4c4));
+ tree.addChunks(blockID5, Arrays.asList(b5c1, b5c2, b5c3, b5c4));
return tree;
}
+ public static Pair<ContainerMerkleTree,
ContainerChecksumTreeManager.ContainerDiff> buildTestTreeWithMismatches(
Review Comment:
We need some javadoc here to explain the return type and interpretation of
the parameters.
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java:
##########
@@ -131,21 +141,221 @@ public static ContainerMerkleTree
buildTestTree(ConfigurationSource conf) {
final long blockID1 = 1;
final long blockID2 = 2;
final long blockID3 = 3;
+ final long blockID4 = 4;
+ final long blockID5 = 5;
ContainerProtos.ChunkInfo b1c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{1, 2, 3}));
ContainerProtos.ChunkInfo b1c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{4, 5, 6}));
- ContainerProtos.ChunkInfo b2c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{7, 8, 9}));
- ContainerProtos.ChunkInfo b2c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{12, 11, 10}));
- ContainerProtos.ChunkInfo b3c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{13, 14, 15}));
- ContainerProtos.ChunkInfo b3c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{16, 17, 18}));
+ ContainerProtos.ChunkInfo b1c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{7, 8, 9}));
+ ContainerProtos.ChunkInfo b1c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{12, 11, 10}));
+ ContainerProtos.ChunkInfo b2c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{13, 14, 15}));
+ ContainerProtos.ChunkInfo b2c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{16, 17, 18}));
+ ContainerProtos.ChunkInfo b2c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{19, 20, 21}));
+ ContainerProtos.ChunkInfo b2c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{22, 23, 24}));
+ ContainerProtos.ChunkInfo b3c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{25, 26, 27}));
+ ContainerProtos.ChunkInfo b3c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{28, 29, 30}));
+ ContainerProtos.ChunkInfo b3c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{31, 32, 33}));
+ ContainerProtos.ChunkInfo b3c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{34, 35, 36}));
+ ContainerProtos.ChunkInfo b4c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{37, 38, 29}));
+ ContainerProtos.ChunkInfo b4c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{40, 41, 42}));
+ ContainerProtos.ChunkInfo b4c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{43, 44, 45}));
+ ContainerProtos.ChunkInfo b4c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{46, 47, 48}));
+ ContainerProtos.ChunkInfo b5c1 = buildChunk(conf, 0, ByteBuffer.wrap(new
byte[]{49, 50, 51}));
+ ContainerProtos.ChunkInfo b5c2 = buildChunk(conf, 1, ByteBuffer.wrap(new
byte[]{52, 53, 54}));
+ ContainerProtos.ChunkInfo b5c3 = buildChunk(conf, 2, ByteBuffer.wrap(new
byte[]{55, 56, 57}));
+ ContainerProtos.ChunkInfo b5c4 = buildChunk(conf, 3, ByteBuffer.wrap(new
byte[]{58, 59, 60}));
ContainerMerkleTree tree = new ContainerMerkleTree();
- tree.addChunks(blockID1, Arrays.asList(b1c1, b1c2));
- tree.addChunks(blockID2, Arrays.asList(b2c1, b2c2));
- tree.addChunks(blockID3, Arrays.asList(b3c1, b3c2));
+ tree.addChunks(blockID1, Arrays.asList(b1c1, b1c2, b1c3, b1c4));
+ tree.addChunks(blockID2, Arrays.asList(b2c1, b2c2, b2c3, b2c4));
+ tree.addChunks(blockID3, Arrays.asList(b3c1, b3c2, b3c3, b3c4));
+ tree.addChunks(blockID4, Arrays.asList(b4c1, b4c2, b4c3, b4c4));
+ tree.addChunks(blockID5, Arrays.asList(b5c1, b5c2, b5c3, b5c4));
return tree;
}
+ public static Pair<ContainerMerkleTree,
ContainerChecksumTreeManager.ContainerDiff> buildTestTreeWithMismatches(
+ ConfigurationSource conf, int numMissingBlocks, int numMissingChunks,
int numCorruptChunks) {
+
+ ContainerMerkleTree tree = buildTestTree(conf);
Review Comment:
Rather than have an implicit dependency between the trees returned by this
method and `buildTestTree` it might be more intuitive if this method takes the
original tree, makes a deep copy, and modifies it. If we operate on the
protobuf instead of `ContainerMerkleTree` then `toBuilder().build()` will
quickly make a deep copy for us.
The `diff` method already operates on protobufs so IMO this method should as
well. This way we can remove all the new public modifiers in the
`ContainerMerkleTree` that are only being used for testing.
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java:
##########
@@ -299,6 +319,64 @@ public void testEmptyFile() throws Exception {
assertEquals(CONTAINER_ID, info.getContainerID());
}
+ @Test
+ public void testContainerWithNoDiff() throws IOException {
Review Comment:
Is this the same test as running one of the parameterized ones with (0, 0,
0)?
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java:
##########
@@ -299,6 +319,64 @@ public void testEmptyFile() throws Exception {
assertEquals(CONTAINER_ID, info.getContainerID());
}
+ @Test
+ public void testContainerWithNoDiff() throws IOException {
+ ContainerMerkleTree ourMerkleTree = buildTestTree(config);
+ ContainerMerkleTree peerMerkleTree = buildTestTree(config);
+ checksumManager.writeContainerDataTree(container, ourMerkleTree);
+ ContainerChecksumTreeManager containerChecksumTreeManager = new
ContainerChecksumTreeManager(
+ new OzoneConfiguration());
+ ContainerProtos.ContainerChecksumInfo peerChecksumInfo =
ContainerProtos.ContainerChecksumInfo.newBuilder()
+ .setContainerID(container.getContainerID())
+ .setContainerMerkleTree(peerMerkleTree.toProto()).build();
+ ContainerChecksumTreeManager.ContainerDiff diff =
+ containerChecksumTreeManager.diff(container, peerChecksumInfo);
Review Comment:
nit. Line length is 120 now, you might need to update your IDE settings.
--
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]
