HDDS-5554. Option to disable checksum verification.#2522
HDDS-5554. Option to disable checksum verification.#2522vivekratnavel merged 7 commits intoapache:masterfrom
Conversation
vivekratnavel
left a comment
vivekratnavel
left a comment
There was a problem hiding this comment.
Overall LGTM. Minor comments inline.
There was a problem hiding this comment.
Can we move this to a common method to get kvData since it is being used by the "testIncorrectChecksum" method as well?
There was a problem hiding this comment.
Can we add "verification" to the config key?
| "hdds.container.checksum.enabled"; | |
| "hdds.container.checksum.verification.enabled"; |
There was a problem hiding this comment.
| public static final String HDDS_CONTAINER_CHECKSUM_ENABLED = | |
| public static final String HDDS_CONTAINER_CHECKSUM_VERIFICATION_ENABLED = |
|
Thank you for the review @vivekratnavel . I will look into it and make the changes |
avijayanhwx
left a comment
avijayanhwx
left a comment
There was a problem hiding this comment.
Thanks @aswinshakil. Approach looks good to me. One comment inline.
.../src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java
Outdated
Show resolved
Hide resolved
avijayanhwx
left a comment
avijayanhwx
left a comment
There was a problem hiding this comment.
Thanks for working on this @aswinshakil. LGTM +1
There was a problem hiding this comment.
nit. can we remove this printStackTrace?
There was a problem hiding this comment.
Sure. I can remove it.
There was a problem hiding this 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 {
There was a problem hiding this comment.
Thank you for the input. I will fixed it.
|
@aswinshakil Thanks for the contribution! @avijayanhwx @ayushtkn Thanks for the reviews! |
4 participants
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