sodonnel commented on code in PR #6520:
URL: https://github.com/apache/ozone/pull/6520#discussion_r1567129578


##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/ECBlockOutputStream.java:
##########
@@ -115,20 +115,43 @@ ContainerCommandResponseProto> executePutBlock(boolean 
close,
     }
 
     BlockData checksumBlockData = null;
+    boolean foundStripeChecksum = false;
+    BlockID blockID = null;
     //Reverse Traversal as all parity will have checksumBytes
     for (int i = blockData.length - 1; i >= 0; i--) {
       BlockData bd = blockData[i];
       if (bd == null) {
         continue;
       }
+      if (blockID == null) {
+        // store the BlockID for logging
+        blockID = bd.getBlockID();
+      }
       List<ChunkInfo> chunks = bd.getChunks();
-      if (chunks != null && chunks.size() > 0 && chunks.get(0)
-          .hasStripeChecksum()) {
-        checksumBlockData = bd;
-        break;
+      if (chunks != null && chunks.size() > 0) {

Review Comment:
   The original logic had:
   
   ```
   if (chunks != null && chunks.size() > 0 && chunks.get(0)
             .hasStripeChecksum()) {
   ```
   
   So that `checksumBlockData` is only set if `hasStripeChecksum())` returns 
true. With the change you have made, `checksumBlockData` will be set if 
`hasChecksumData()`, but it might not have stripeChecksum in it.
   
   Then at line 155 it will enter the IF block and at line 174 I am not sure 
what will happen if stripeCheckSum is missing.
   
   All the IF statement at 155 does is copy in the stripeChecksum if it exists. 
So if it does not exist, there is no point in going into that IF at all, as we 
will just be copying the original chunkChecksum out and back in again.
   
   Following on from this - we only set `checksumBlockData` if there is a 
stripeChecksum, then you can also remove the `foundStripeChecksum` boolean as 
`checksumBlockData != null` means the same thing.



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