You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/08/10 16:52:39 UTC
[GitHub] [ozone] aswinshakil opened a new pull request #2522: HDDS-5554. Option to disable checksum verification.
aswinshakil opened a new pull request #2522:
URL: https://github.com/apache/ozone/pull/2522
## What changes were proposed in this pull request?
To provide an option to disable checksum verification on container files on startup.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5554
## How was this patch tested?
This patch was tested using testDisabledChecksum Unit test and GitHub CI tests.
https://github.com/aswinshakil/ozone/actions/runs/1114468266
--
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
[GitHub] [ozone] aswinshakil commented on a change in pull request #2522: HDDS-5554. Option to disable checksum verification.
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on a change in pull request #2522:
URL: https://github.com/apache/ozone/pull/2522#discussion_r690627694
##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDataYaml.java
##########
@@ -233,28 +236,47 @@ public void testChecksumInContainerFile() throws IOException {
// Read from .container file, and verify data.
KeyValueContainerData kvData = (KeyValueContainerData) ContainerDataYaml
.readContainerFile(containerFile);
- ContainerUtils.verifyChecksum(kvData);
+ ContainerUtils.verifyChecksum(kvData, conf);
cleanup();
}
+ private KeyValueContainerData getKeyValueContainerData() throws IOException {
+ String containerFile = "incorrect.checksum.container";
+ //Get file from resources folder
+ ClassLoader classLoader = getClass().getClassLoader();
+ File file = new File(classLoader.getResource(containerFile).getFile());
+ return (KeyValueContainerData) ContainerDataYaml.readContainerFile(file);
+ }
+
/**
* Test to verify incorrect checksum is detected.
*/
@Test
public void testIncorrectChecksum() {
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);
- ContainerUtils.verifyChecksum(kvData);
+ KeyValueContainerData kvData = getKeyValueContainerData();
+ 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 {
+ KeyValueContainerData kvData = getKeyValueContainerData();
+ conf.setBoolean(HddsConfigKeys.
+ HDDS_CONTAINER_CHECKSUM_VERIFICATION_ENABLED, false);
+ ContainerUtils.verifyChecksum(kvData, conf);
+ } catch (Exception ex) {
+ fail("testDisabledChecksum failed");
+ }
Review comment:
Thank you for the input. I will fixed it.
--
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
[GitHub] [ozone] vivekratnavel merged pull request #2522: HDDS-5554. Option to disable checksum verification.
Posted by GitBox <gi...@apache.org>.
vivekratnavel merged pull request #2522:
URL: https://github.com/apache/ozone/pull/2522
--
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
[GitHub] [ozone] aswinshakil commented on a change in pull request #2522: HDDS-5554. Option to disable checksum verification.
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on a change in pull request #2522:
URL: https://github.com/apache/ozone/pull/2522#discussion_r690627694
##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDataYaml.java
##########
@@ -233,28 +236,47 @@ public void testChecksumInContainerFile() throws IOException {
// Read from .container file, and verify data.
KeyValueContainerData kvData = (KeyValueContainerData) ContainerDataYaml
.readContainerFile(containerFile);
- ContainerUtils.verifyChecksum(kvData);
+ ContainerUtils.verifyChecksum(kvData, conf);
cleanup();
}
+ private KeyValueContainerData getKeyValueContainerData() throws IOException {
+ String containerFile = "incorrect.checksum.container";
+ //Get file from resources folder
+ ClassLoader classLoader = getClass().getClassLoader();
+ File file = new File(classLoader.getResource(containerFile).getFile());
+ return (KeyValueContainerData) ContainerDataYaml.readContainerFile(file);
+ }
+
/**
* Test to verify incorrect checksum is detected.
*/
@Test
public void testIncorrectChecksum() {
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);
- ContainerUtils.verifyChecksum(kvData);
+ KeyValueContainerData kvData = getKeyValueContainerData();
+ 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 {
+ KeyValueContainerData kvData = getKeyValueContainerData();
+ conf.setBoolean(HddsConfigKeys.
+ HDDS_CONTAINER_CHECKSUM_VERIFICATION_ENABLED, false);
+ ContainerUtils.verifyChecksum(kvData, conf);
+ } catch (Exception ex) {
+ fail("testDisabledChecksum failed");
+ }
Review comment:
Thank you for the input. I will fixed it.
--
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
[GitHub] [ozone] aswinshakil commented on a change in pull request #2522: HDDS-5554. Option to disable checksum verification.
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on a change in pull request #2522:
URL: https://github.com/apache/ozone/pull/2522#discussion_r686963320
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java
##########
@@ -169,7 +169,7 @@ public static void parseKVContainerData(KeyValueContainerData kvContainerData,
File metadataPath = new File(kvContainerData.getMetadataPath());
// Verify Checksum
- ContainerUtils.verifyChecksum(kvContainerData);
+ ContainerUtils.verifyChecksum(kvContainerData, config);
Review comment:
I thought the checksum verification enable/ disable can be inside the verifychecksum function, so that every time we invoke the function we don't have to enable/disable outside the function call. What are your thoughts on that?
--
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
[GitHub] [ozone] ayushtkn commented on a change in pull request #2522: HDDS-5554. Option to disable checksum verification.
Posted by GitBox <gi...@apache.org>.
ayushtkn commented on a change in pull request #2522:
URL: https://github.com/apache/ozone/pull/2522#discussion_r690036167
##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDataYaml.java
##########
@@ -233,28 +236,47 @@ public void testChecksumInContainerFile() throws IOException {
// Read from .container file, and verify data.
KeyValueContainerData kvData = (KeyValueContainerData) ContainerDataYaml
.readContainerFile(containerFile);
- ContainerUtils.verifyChecksum(kvData);
+ ContainerUtils.verifyChecksum(kvData, conf);
cleanup();
}
+ private KeyValueContainerData getKeyValueContainerData() throws IOException {
+ String containerFile = "incorrect.checksum.container";
+ //Get file from resources folder
+ ClassLoader classLoader = getClass().getClassLoader();
+ File file = new File(classLoader.getResource(containerFile).getFile());
+ return (KeyValueContainerData) ContainerDataYaml.readContainerFile(file);
+ }
+
/**
* Test to verify incorrect checksum is detected.
*/
@Test
public void testIncorrectChecksum() {
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);
- ContainerUtils.verifyChecksum(kvData);
+ KeyValueContainerData kvData = getKeyValueContainerData();
+ 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 {
+ KeyValueContainerData kvData = getKeyValueContainerData();
+ conf.setBoolean(HddsConfigKeys.
+ HDDS_CONTAINER_CHECKSUM_VERIFICATION_ENABLED, false);
+ ContainerUtils.verifyChecksum(kvData, conf);
+ } catch (Exception ex) {
+ fail("testDisabledChecksum failed");
+ }
Review comment:
why you need a try-catch?
You can remove the try catch and add throws to the method signature ``public void testDisabledChecksum() throws Exception {``
##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDataYaml.java
##########
@@ -233,28 +236,47 @@ public void testChecksumInContainerFile() throws IOException {
// Read from .container file, and verify data.
KeyValueContainerData kvData = (KeyValueContainerData) ContainerDataYaml
.readContainerFile(containerFile);
- ContainerUtils.verifyChecksum(kvData);
+ ContainerUtils.verifyChecksum(kvData, conf);
cleanup();
}
+ private KeyValueContainerData getKeyValueContainerData() throws IOException {
+ String containerFile = "incorrect.checksum.container";
+ //Get file from resources folder
+ ClassLoader classLoader = getClass().getClassLoader();
+ File file = new File(classLoader.getResource(containerFile).getFile());
+ return (KeyValueContainerData) ContainerDataYaml.readContainerFile(file);
+ }
+
/**
* Test to verify incorrect checksum is detected.
*/
@Test
public void testIncorrectChecksum() {
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);
- ContainerUtils.verifyChecksum(kvData);
+ KeyValueContainerData kvData = getKeyValueContainerData();
+ 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 {
+ KeyValueContainerData kvData = getKeyValueContainerData();
+ conf.setBoolean(HddsConfigKeys.
+ HDDS_CONTAINER_CHECKSUM_VERIFICATION_ENABLED, false);
+ ContainerUtils.verifyChecksum(kvData, conf);
+ } catch (Exception ex) {
+ fail("testDisabledChecksum failed");
+ }
+
Review comment:
nit: avoid this empty line
--
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
[GitHub] [ozone] avijayanhwx commented on a change in pull request #2522: HDDS-5554. Option to disable checksum verification.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2522:
URL: https://github.com/apache/ozone/pull/2522#discussion_r689709446
##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDataYaml.java
##########
@@ -233,28 +236,48 @@ public void testChecksumInContainerFile() throws IOException {
// Read from .container file, and verify data.
KeyValueContainerData kvData = (KeyValueContainerData) ContainerDataYaml
.readContainerFile(containerFile);
- ContainerUtils.verifyChecksum(kvData);
+ ContainerUtils.verifyChecksum(kvData, conf);
cleanup();
}
+ private KeyValueContainerData getKeyValueContainerData() throws IOException {
+ String containerFile = "incorrect.checksum.container";
+ //Get file from resources folder
+ ClassLoader classLoader = getClass().getClassLoader();
+ File file = new File(classLoader.getResource(containerFile).getFile());
+ return (KeyValueContainerData) ContainerDataYaml.readContainerFile(file);
+ }
+
/**
* Test to verify incorrect checksum is detected.
*/
@Test
public void testIncorrectChecksum() {
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);
- ContainerUtils.verifyChecksum(kvData);
+ KeyValueContainerData kvData = getKeyValueContainerData();
+ 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 {
+ KeyValueContainerData kvData = getKeyValueContainerData();
+ conf.setBoolean(HddsConfigKeys.
+ HDDS_CONTAINER_CHECKSUM_VERIFICATION_ENABLED, false);
+ ContainerUtils.verifyChecksum(kvData, conf);
+ } catch (Exception ex) {
+ ex.printStackTrace();
Review comment:
nit. can we remove this printStackTrace?
--
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
[GitHub] [ozone] aswinshakil commented on a change in pull request #2522: HDDS-5554. Option to disable checksum verification.
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on a change in pull request #2522:
URL: https://github.com/apache/ozone/pull/2522#discussion_r689712643
##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDataYaml.java
##########
@@ -233,28 +236,48 @@ public void testChecksumInContainerFile() throws IOException {
// Read from .container file, and verify data.
KeyValueContainerData kvData = (KeyValueContainerData) ContainerDataYaml
.readContainerFile(containerFile);
- ContainerUtils.verifyChecksum(kvData);
+ ContainerUtils.verifyChecksum(kvData, conf);
cleanup();
}
+ private KeyValueContainerData getKeyValueContainerData() throws IOException {
+ String containerFile = "incorrect.checksum.container";
+ //Get file from resources folder
+ ClassLoader classLoader = getClass().getClassLoader();
+ File file = new File(classLoader.getResource(containerFile).getFile());
+ return (KeyValueContainerData) ContainerDataYaml.readContainerFile(file);
+ }
+
/**
* Test to verify incorrect checksum is detected.
*/
@Test
public void testIncorrectChecksum() {
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);
- ContainerUtils.verifyChecksum(kvData);
+ KeyValueContainerData kvData = getKeyValueContainerData();
+ 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 {
+ KeyValueContainerData kvData = getKeyValueContainerData();
+ conf.setBoolean(HddsConfigKeys.
+ HDDS_CONTAINER_CHECKSUM_VERIFICATION_ENABLED, false);
+ ContainerUtils.verifyChecksum(kvData, conf);
+ } catch (Exception ex) {
+ ex.printStackTrace();
Review comment:
Sure. I can remove it.
--
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
[GitHub] [ozone] aswinshakil commented on pull request #2522: HDDS-5554. Option to disable checksum verification.
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on pull request #2522:
URL: https://github.com/apache/ozone/pull/2522#issuecomment-896222723
Thank you for the review @vivekratnavel . I will look into it and make the changes
--
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
[GitHub] [ozone] vivekratnavel commented on a change in pull request #2522: HDDS-5554. Option to disable checksum verification.
Posted by 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
[GitHub] [ozone] avijayanhwx commented on a change in pull request #2522: HDDS-5554. Option to disable checksum verification.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2522:
URL: https://github.com/apache/ozone/pull/2522#discussion_r686505043
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java
##########
@@ -169,7 +169,7 @@ public static void parseKVContainerData(KeyValueContainerData kvContainerData,
File metadataPath = new File(kvContainerData.getMetadataPath());
// Verify Checksum
- ContainerUtils.verifyChecksum(kvContainerData);
+ ContainerUtils.verifyChecksum(kvContainerData, config);
Review comment:
Any reason why moved the config check into the method? If we keep it here, it is more readable.
`if (enabled) {
verifyChecksum
}`
What do you think?
--
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
[GitHub] [ozone] vivekratnavel commented on pull request #2522: HDDS-5554. Option to disable checksum verification.
Posted by GitBox <gi...@apache.org>.
vivekratnavel commented on pull request #2522:
URL: https://github.com/apache/ozone/pull/2522#issuecomment-904374528
@aswinshakil Thanks for the contribution! @avijayanhwx @ayushtkn Thanks for the reviews!
--
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