[GitHub] [ozone] vivekratnavel commented on a change in pull request #2522: HDDS-5554. Option to disable checksum verification.

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

vivekratnavel commented on a change in pull request #2522:
URL: https://github.com/apache/ozone/pull/2522#discussion_r686184630



##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDataYaml.java
##########
@@ -250,11 +253,32 @@ public void testIncorrectChecksum() {
       File file = new File(classLoader.getResource(containerFile).getFile());
       KeyValueContainerData kvData = (KeyValueContainerData) ContainerDataYaml
           .readContainerFile(file);
-      ContainerUtils.verifyChecksum(kvData);
+      ContainerUtils.verifyChecksum(kvData, conf);
       fail("testIncorrectChecksum failed");
     } catch (Exception ex) {
       GenericTestUtils.assertExceptionContains("Container checksum error for " +
           "ContainerID:", ex);
     }
   }
+
+  /**
+   * Test to verify disabled checksum with incorrect checksum.
+   */
+  @Test
+  public void testDisabledChecksum(){
+    try {
+      String containerFile = "incorrect.checksum.container";
+      //Get file from resources folder
+      ClassLoader classLoader = getClass().getClassLoader();
+      File file = new File(classLoader.getResource(containerFile).getFile());
+      KeyValueContainerData kvData = (KeyValueContainerData) ContainerDataYaml
+              .readContainerFile(file);

Review comment:
       Can we move this to a common method to get kvData since it is being used by the "testIncorrectChecksum" method as well?
   

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsConfigKeys.java
##########
@@ -265,4 +265,8 @@ private HddsConfigKeys() {
       "hdds.datanode.ratis.server.request.timeout";
   public static final String
       HDDS_DATANODE_RATIS_SERVER_REQUEST_TIMEOUT_DEFAULT = "2m";
+  public static final String HDDS_CONTAINER_CHECKSUM_ENABLED =
+          "hdds.container.checksum.enabled";

Review comment:
       Can we add "verification" to the config key?
   ```suggestion
             "hdds.container.checksum.verification.enabled";
   ```

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsConfigKeys.java
##########
@@ -265,4 +265,8 @@ private HddsConfigKeys() {
       "hdds.datanode.ratis.server.request.timeout";
   public static final String
       HDDS_DATANODE_RATIS_SERVER_REQUEST_TIMEOUT_DEFAULT = "2m";
+  public static final String HDDS_CONTAINER_CHECKSUM_ENABLED =

Review comment:
       ```suggestion
     public static final String HDDS_CONTAINER_CHECKSUM_VERIFICATION_ENABLED =
   ```

##########
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##########
@@ -2832,4 +2832,13 @@
     <description>The S3Gateway service principal.
       Ex: s3g/_HOST@REALM.COM</description>
   </property>
+
+  <property>
+    <name>hdds.container.checksum.enabled</name>

Review comment:
       Same as above.




-- 
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: issues-unsubscribe@ozone.apache.org

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



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