errose28 commented on code in PR #8204:
URL: https://github.com/apache/ozone/pull/8204#discussion_r2042597464


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java:
##########
@@ -383,6 +373,24 @@ public ByteString 
getContainerChecksumInfo(KeyValueContainerData data) throws IO
     }
   }
 
+  public static Optional<ContainerProtos.ContainerChecksumInfo> 
readChecksumInfo(ContainerData data)

Review Comment:
   Let's add a comment here similar to what is on `read` indicating that 
callers don't need to worry about synchronization. This method is now also part 
of the public API so it should have some javadoc.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java:
##########
@@ -498,7 +503,97 @@ public void testContainerChecksumChunkCorruption() throws 
Exception {
     ContainerProtos.ContainerChecksumInfo newContainerChecksumInfo = 
readChecksumFile(container.getContainerData());
     
assertTreesSortedAndMatch(oldContainerChecksumInfo.getContainerMerkleTree(),
         newContainerChecksumInfo.getContainerMerkleTree());
-    Assertions.assertEquals(oldDataChecksum, 
newContainerChecksumInfo.getContainerMerkleTree().getDataChecksum());
+    assertEquals(oldDataChecksum, 
newContainerChecksumInfo.getContainerMerkleTree().getDataChecksum());
+    TestHelper.validateData(KEY_NAME, data, store, volume, bucket);
+  }
+
+  @Test
+  public void testDataChecksumReportedAtSCM() throws Exception {
+    // 1. Write data to a container.
+    // Read the key back and check its hash.
+    String volume = UUID.randomUUID().toString();
+    String bucket = UUID.randomUUID().toString();
+    Pair<Long, byte[]> containerAndData = getDataAndContainer(true, 20 * 1024 
* 1024, volume, bucket);
+    long containerID = containerAndData.getLeft();
+    byte[] data = containerAndData.getRight();
+    // Get the datanodes where the container replicas are stored.
+    List<DatanodeDetails> dataNodeDetails = 
cluster.getStorageContainerManager().getContainerManager()
+        .getContainerReplicas(ContainerID.valueOf(containerID))
+        .stream().map(ContainerReplica::getDatanodeDetails)
+        .collect(Collectors.toList());
+    assertEquals(3, dataNodeDetails.size());
+    HddsDatanodeService hddsDatanodeService = 
cluster.getHddsDatanode(dataNodeDetails.get(0));
+    DatanodeStateMachine datanodeStateMachine = 
hddsDatanodeService.getDatanodeStateMachine();
+    Container<?> container = 
datanodeStateMachine.getContainer().getContainerSet().getContainer(containerID);
+    KeyValueContainerData containerData = (KeyValueContainerData) 
container.getContainerData();
+    ContainerProtos.ContainerChecksumInfo oldContainerChecksumInfo = 
readChecksumFile(container.getContainerData());
+    KeyValueHandler kvHandler = (KeyValueHandler) 
datanodeStateMachine.getContainer().getDispatcher()
+        .getHandler(ContainerProtos.ContainerType.KeyValueContainer);
+
+    BlockManager blockManager = kvHandler.getBlockManager();
+    List<BlockData> blockDataList = blockManager.listBlock(container, -1, 100);

Review Comment:
   nit. declare and init this and `chunksPath` around line 545 where it is 
actually used.



##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java:
##########
@@ -201,7 +201,6 @@ public void testDeletedBlocksPreservedOnTreeWrite() throws 
Exception {
     
assertEquals(metrics.getReadContainerMerkleTreeLatencyNS().lastStat().total(), 
0);
     List<Long> expectedBlocksToDelete = Arrays.asList(1L, 2L, 3L);
     checksumManager.markBlocksAsDeleted(container, new 
ArrayList<>(expectedBlocksToDelete));
-    
assertEquals(metrics.getReadContainerMerkleTreeLatencyNS().lastStat().total(), 
0);

Review Comment:
   Why were these tests removed?



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