ivandika3 commented on code in PR #10246:
URL: https://github.com/apache/ozone/pull/10246#discussion_r3230929093


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/ContainerUtils.java:
##########
@@ -246,11 +247,15 @@ public static void 
verifyContainerFileChecksum(ContainerData containerData,
                     HDDS_CONTAINER_CHECKSUM_VERIFICATION_ENABLED_DEFAULT);
     if (enabled) {
       String storedChecksum = containerData.getContainerFileChecksum();
+      StorageType storageType =
+          containerData instanceof KeyValueContainerData
+              ? containerData.getStorageType() : null;
 
       Yaml yaml = ContainerDataYaml.getYamlForContainerType(
           containerData.getContainerType(),
-          containerData instanceof KeyValueContainerData &&
-              ((KeyValueContainerData)containerData).getReplicaIndex() > 0);
+          containerData instanceof KeyValueContainerData
+              && ((KeyValueContainerData) containerData).getReplicaIndex() > 0,
+          storageType);

Review Comment:
   Internally, we already removed this checksum logic for rollback 
compatibility. Please check this.
   
   > Scenarios that cause checksum verification failure after rollback:
   > Ozone calculates the checksum of Container YAML through a process of 
"reading YAML string → deserializing to ContainerDataYaml → reserializing to a 
YAML string → calculating the checksum," rather than directly using the 
original YAML string.
   > When the new version of the YAML file contains a StorageType field, the 
old version will lose this field during this conversion process, resulting in 
inconsistencies between the reserialized YAML content and the original file, 
thus causing checksum mismatch.
   > 
   > soluation
   > We do not include the StorageType for Container checksum calculation.



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/ContainerUtils.java:
##########
@@ -246,11 +247,15 @@ public static void 
verifyContainerFileChecksum(ContainerData containerData,
                     HDDS_CONTAINER_CHECKSUM_VERIFICATION_ENABLED_DEFAULT);
     if (enabled) {
       String storedChecksum = containerData.getContainerFileChecksum();
+      StorageType storageType =
+          containerData instanceof KeyValueContainerData
+              ? containerData.getStorageType() : null;
 
       Yaml yaml = ContainerDataYaml.getYamlForContainerType(
           containerData.getContainerType(),
-          containerData instanceof KeyValueContainerData &&
-              ((KeyValueContainerData)containerData).getReplicaIndex() > 0);
+          containerData instanceof KeyValueContainerData
+              && ((KeyValueContainerData) containerData).getReplicaIndex() > 0,
+          storageType);

Review Comment:
   Internally, we already removed this checksum logic for rollback 
compatibility. Please help to check this.
   
   > Scenarios that cause checksum verification failure after rollback:
   > Ozone calculates the checksum of Container YAML through a process of 
"reading YAML string → deserializing to ContainerDataYaml → reserializing to a 
YAML string → calculating the checksum," rather than directly using the 
original YAML string.
   > When the new version of the YAML file contains a StorageType field, the 
old version will lose this field during this conversion process, resulting in 
inconsistencies between the reserialized YAML content and the original file, 
thus causing checksum mismatch.
   > 
   > soluation
   > We do not include the StorageType for Container checksum calculation.



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