[GitHub] [hadoop] ZanderXu commented on a diff in pull request #4155: HDFS-16533. COMPOSITE_CRC failed between replicated file and striped …

[GitBox <gi...@apache.org>]

ZanderXu commented on code in PR #4155:
URL: https://github.com/apache/hadoop/pull/4155#discussion_r874941317


##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/FileChecksumHelper.java:
##########
@@ -316,18 +317,22 @@ FileChecksum makeCompositeCrcResult() throws IOException {
             "Added blockCrc 0x{} for block index {} of size {}",
             Integer.toString(blockCrc, 16), i, block.getBlockSize());
       }
-
-      // NB: In some cases the located blocks have their block size adjusted
-      // explicitly based on the requested length, but not all cases;
-      // these numbers may or may not reflect actual sizes on disk.
-      long reportedLastBlockSize =
-          blockLocations.getLastLocatedBlock().getBlockSize();
-      long consumedLastBlockLength = reportedLastBlockSize;
-      if (length - sumBlockLengths < reportedLastBlockSize) {
-        LOG.warn(
-            "Last block length {} is less than reportedLastBlockSize {}",
-            length - sumBlockLengths, reportedLastBlockSize);
-        consumedLastBlockLength = length - sumBlockLengths;
+      LocatedBlock nextBlock = locatedBlocks.get(i);
+      long consumedLastBlockLength = Math.min(length - sumBlockLengths,
+          nextBlock.getBlockSize());
+      LocatedBlock lastBlock = blockLocations.getLastLocatedBlock();
+      if (nextBlock.equals(lastBlock)) {

Review Comment:
   Thanks @jojochuang for your comment. 
   First, i will explain the goal of UT:
   
   1. Use the same context to create a replicated file and a striped file.
   2. Set the conf the use COMPOSITE_CRC
   3. Expected  the same checksum result of any length from the replicated file and the striped file via getFileChecksum
   
   Second, i will explain the root cause:
   
   1. blockLocations in line 104, contains a blocks and a lastLocatedBlock
   2. the last block in blocks maybe not the same with lastLocatedBlock when the input length is less than file length.
   3. so, we cannot always compare with the lastLocatedBlock to get the composer length in line 336.



-- 
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-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org