HDDS-6086. Compute MD5MD5CRC file checksum using chunk checksums from DataNodes #2919

jojochuang · 2021-12-13T08:29:05Z

What changes were proposed in this pull request?

This is an initial draft to create the basic framework for fetching file checksums.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-6086

How was this patch tested?

A unit test is added to test FileChecksumHelper

rakeshadr

Thanks @jojochuang for the good work!

Added few comments, please take a look at it.

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/BaseFileChecksumHelper.java

...-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ReplicatedFileChecksumHelper.java

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/BaseFileChecksumHelper.java

...ne/client/src/test/java/org/apache/hadoop/ozone/client/TestReplicatedFileChecksumHelper.java

rakeshadr · 2021-12-16T12:02:03Z

@jojochuang There are few checkstyle errors, pls take care.

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ReplicatedFileChecksumHelper.java
 38: Missing a Javadoc comment.
 43: Line is longer than 80 characters (found 97).
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/BaseFileChecksumHelper.java
 50: Variable &apos;rpcClient&apos; must be private and have accessor methods.
 51: Variable &apos;xceiverClientFactory&apos; must be private and have accessor methods.
 55: Variable &apos;blockChecksumBuf&apos; must be private and have accessor methods.
 58: Variable &apos;keyLocationInfos&apos; must be private and have accessor methods.

...-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ReplicatedFileChecksumHelper.java

...ne/client/src/test/java/org/apache/hadoop/ozone/client/TestReplicatedFileChecksumHelper.java

...-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ReplicatedFileChecksumHelper.java

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/BaseFileChecksumHelper.java

rakeshadr

Overall the updated patch looks fine. Added few suggestions/comments. Also, there is pom.xml conflict, please take care. Thanks!

Change-Id: I05320e17c27f8b1cb3976d7021b1768a4def0549

Change-Id: I3e0bf88f950be487aafa6559e8e818b104ebd43d

Change-Id: Iebd6e2b1ba106bc079e875d400ed19d6fa1629ec

Change-Id: I5ca7a4a81d0fb623f58a27699a7da5bbcc83b1a2

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/BaseFileChecksumHelper.java

rakeshadr · 2022-01-05T06:57:00Z

...ne/client/src/test/java/org/apache/hadoop/ozone/client/TestReplicatedFileChecksumHelper.java

Thanks for the good test case. Since this is primary patch I agree to keep it simple here and push this patch in asap.

Adding a note here for including more test cases in follow-up jira tasks. I'd to to add more test cases with variable length values(-1, 0, >0), with more blocks.

hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java

jojochuang · 2022-01-06T09:30:07Z

the test failure looks related. I'm looking into it.

jojochuang · 2022-01-07T05:29:07Z

I'm reverting back to commit da9c22c. The refactor to create a helper method is causing more trouble than i had expected. I'll defer that change to a separate PR @rakeshadr if you are okay with that.

lokeshj1703

@xiaoyuyao Thanks for working on this! The changes look good to me. I have couple of questions.

lokeshj1703 · 2022-01-07T07:32:23Z

...-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ReplicatedFileChecksumHelper.java

+    if (getRemaining() < blockNumBytes) {
+      blockNumBytes = getRemaining();
+    }
+    setRemaining(getRemaining() - blockNumBytes);


What is the role of remaining?

We obtain the the metadata of blocks after blocks, until remaining is < 0. Remaining is decremented by the block length each time.

It seems we are not setting remaining to key length initially.

...-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ReplicatedFileChecksumHelper.java

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/BaseFileChecksumHelper.java

rakeshadr · 2022-01-07T08:54:41Z

I'm reverting back to commit da9c22c. The refactor to create a helper method is causing more trouble than i had expected. I'll defer that change to a separate PR @rakeshadr if you are okay with that.

@jojochuang Yes, makes sense to me.

rakeshadr

Thanks @jojochuang, +1 LGTM

Please handle the comment to add more unit test cases in later sub-tasks.

jojochuang · 2022-01-10T01:04:17Z

Thanks Rakesh. Will adding additional UT in later PRs.

adoroszlai · 2022-01-10T08:09:01Z

hadoop-ozone/client/src/test/resources/log4j.properties

-log4j.rootLogger=INFO,stdout
-log4j.threshold=ALL
-log4j.appender.stdout=org.apache.log4j.ConsoleAppender
-log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
-log4j.appender.stdout.layout.ConversionPattern=%d{ISO8601} [%t] %-5p %c{2} (%F:%M(%L)) - %m%n


@jojochuang This file was added only recently for HDDS-6108. Why is it removed now?

@REPLicated

…ansport support (#3074) * HDDS-6149. Remove unused keytabs (#2960) * HDDS-6094. Some unit tests are skipped due to using JUnit4 runner (#2909) * HDDS-6075. OzoneConfiguration constructor overrides input configuration keys. (#2921) * HDDS-4177. SCM Container DB bootstrap on Recon startup (#2942) * HDDS-6086. Compute MD5MD5CRC file checksum using chunk checksums from DataNodes (#2919) * HDDS-6148. Validate ContainerBalancerConfiguration before start ContainerBalancer (#2957) * HDDS-6161. SCM StateMachine failing to reinitialize doesn't terminate the process. (#2971) * HDDS-6134. Move replication-specific config to ReplicationServer (#2943) * HDDS-4010. S3G startup fails when multiple service ids are configured. (#2976) * HDDS-6170. Add metrics to replication manager to track container health states (#2975) * HDDS-3231. Cleanup KeyManagerImpl (#2961) * HDDS-5927. Improve defaults in ContainerBalancerConfiguration (#2892) * HDDS-6157. More consistent synchronization in InputStreams (#2965) * HDDS-4743. [FSO] Add FSO variant of ITestOzoneContractDistcp. (#2980) * HDDS-6114. Intermittent error due to Failed to init RocksDB (#2947) * HDDS-6175. Use s3Auth during proxy during decrypt in RpcClient. (#2981) * HDDS-6175. Use s3Auth during proxy during decrypt in RpcClient. * HDDS-5740. Enable ratis by default for SCM. (#2637) * HDDS-6183. Intermittent failure in TestKeyDeletingService.checkIfDeleteServiceWithFailingSCM. (#2991) * HDDS-4190. Intermittent failure in TestOMVolumeSetOwnerRequest and TestOMVolumeSetQuotaRequest. (#2982) * HDDS-6120. Compute block checksum using chunk checksums (#2930) * HDDS-6147. Add ability in OM to get limited delta updates (#2956) * HDDS-6195. Remove unused jmh-core dependency. (#2997) * HDDS-6167. Update ozone-runner version to 20211202-1 (#2969) * HDDS-6171. Create an API to change Bucket Owner (#2988) * HDDS-6163. Fix PATH in docker image (#2967) * HDDS-6202. Avoid using jmh-generator-annprocess since it is GPL2.0. (#2998) * HDDS-6135. SCM Container DB bootstrap on Recon startup for SCM HA. (#2972) * HDDS-6109. Preserve the underlying exception raised in client lib. (#2989) * HDDS-3408. Rename ChunkLayOutVersion to ContainerLayoutVersion. (#2983) * HDDS-6203. CleanUp incomplete gz files during Container move (#3000) * HDDS-6216. Move OMOpenKeysDeleteRequest to package om.request.key (#3011) * HDDS-6191. Intermittent failure in TestDeleteWithSlowFollower (#3015) * HDDS-6128. CLI tool that downloads all the block replicas and creates a manifest file (#2987) * HDDS-6177. Extend container info command to include replica details (#2995) * HDDS-6211. [Docs] Image styling on deployed site does not replicate local builds. (#3007) * HDDS-6219. Switch to RATIS ReplicationType from STAND_ALONE in our tests. (#3014) * HDDS-6192. feature/Observability.md translated to Chinese (#2994) * HDDS-6205. Add CLI command to display the latest Replication Manager report (#3013) * HDDS-6227. Test helpers should observe naming conditions (#3020) * HDDS-6239. ozonesecure-mr failing with No URLs in mirrorlist (#3029) * HDDS-6201. Fix NPE for DataScanner with scanned container deleted by others. (#3005) * HDDS-5529. For any IOexception from @REPLicated method we should throw it (#2788) * HDDS-6181. Change SCMHAInvocationHandler#invokeRatis() logging to TRACE (#2992) * HDDS-6206. Application errors must not flood system log (#3001) * HDDS-6245. Add BucketLayout logging to Audit Logs (#3040) * HDDS-6238 Reduce memory requirements for list keys. (#3032) * HDDS-2919. Intermittent failure in TestRDBStore (#3028) * HDDS-6253. Unnecessary duplicate smoketest after defaulting to FSO (#3036) * HDDS-6204. Cleanup handling malformed authorization header (#2999) * HDDS-6169. Selective checks: skip junit tests on ozone-runner image update (#2974) * HDDS-6270. Use a dedicated file instead of /etc/passwd for xcompat acceptance test (#3050) * HDDS-6273. Amend doc SecuringTDE.md (#3047) * HDDS-6140. Selective checks: skip unit check for integration-test changes (#2948) * HDDS-6215. Recon get limited delta updates from OM (#3009) * HDDS-6125. Recon get limited delta updates from OM * HDDS-6215. Fix unit test * trigger new CI check * HDDS-6215. Fix typo * trigger new CI check Co-authored-by: Symious <[email protected]> * HDDS-6226. Run tests for selective CI checks in CI (#3019) * HDDS-6247. Avoid logging stack trace for user input problems (#3039) * HDDS-6208. New checkstyle: WhitespaceAround (#3003) * HDDS-6289. Upgrade acceptance test log flooded with parse error (#3063) Co-authored-by: Siyao Meng <[email protected]> * Trigger Build * Fix integration test for added configuation field for selecting OmTransport for s3 gateway - TestOzoneConfigurationFields (added config key not in xml). Co-authored-by: Doroszlai, Attila <[email protected]> Co-authored-by: Lokesh Jain <[email protected]> Co-authored-by: Aswin Shakil Balasubramanian <[email protected]> Co-authored-by: Wei-Chiu Chuang <[email protected]> Co-authored-by: Symious <[email protected]> Co-authored-by: Bharat Viswanadham <[email protected]> Co-authored-by: Stephen O'Donnell <[email protected]> Co-authored-by: GeorgeJahad <[email protected]> Co-authored-by: Siddhant Sangwan <[email protected]> Co-authored-by: Jyotinder Singh <[email protected]> Co-authored-by: Shashikant Banerjee <[email protected]> Co-authored-by: Tsz-Wo Nicholas Sze <[email protected]> Co-authored-by: Kiyoshi Mizumaru <[email protected]> Co-authored-by: Ritesh H Shukla <[email protected]> Co-authored-by: Nibiru <[email protected]> Co-authored-by: Kaijie Chen <[email protected]> Co-authored-by: Zita Dombi <[email protected]> Co-authored-by: Istvan Fajth <[email protected]> Co-authored-by: steinsgateted <[email protected]> Co-authored-by: Gui Hecheng <[email protected]> Co-authored-by: Jackson Yao <[email protected]> Co-authored-by: Keyi Song <[email protected]> Co-authored-by: UENISHI Kota <[email protected]> Co-authored-by: Siyao Meng <[email protected]> Co-authored-by: Symious <[email protected]>

jojochuang marked this pull request as ready for review December 15, 2021 09:26

jojochuang requested a review from rakeshadr December 15, 2021 09:26

rakeshadr reviewed Dec 16, 2021

View reviewed changes

jojochuang force-pushed the HDDS-6086 branch from 5c87bee to e3eeae5 Compare December 22, 2021 05:29

rakeshadr reviewed Jan 5, 2022

View reviewed changes

...-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ReplicatedFileChecksumHelper.java Outdated Show resolved Hide resolved

rakeshadr reviewed Jan 5, 2022

View reviewed changes

...-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ReplicatedFileChecksumHelper.java Outdated Show resolved Hide resolved

rakeshadr reviewed Jan 5, 2022

View reviewed changes

...ne/client/src/test/java/org/apache/hadoop/ozone/client/TestReplicatedFileChecksumHelper.java Outdated Show resolved Hide resolved

rakeshadr reviewed Jan 5, 2022

View reviewed changes

...-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ReplicatedFileChecksumHelper.java Outdated Show resolved Hide resolved

rakeshadr reviewed Jan 5, 2022

View reviewed changes

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/BaseFileChecksumHelper.java Outdated Show resolved Hide resolved

rakeshadr reviewed Jan 5, 2022

View reviewed changes

jojochuang added 12 commits January 5, 2022 14:15

Draft

2dd87b8

Change-Id: I05320e17c27f8b1cb3976d7021b1768a4def0549

stash

c18a3ec

Change-Id: I3e0bf88f950be487aafa6559e8e818b104ebd43d

WIP

ba9ac4a

Change-Id: Iebd6e2b1ba106bc079e875d400ed19d6fa1629ec

Fix

b10bf75

Change-Id: I5ca7a4a81d0fb623f58a27699a7da5bbcc83b1a2

Completed initial tests.

585f0ec

Moved out extra code

a825dca

Removed log related code

5efb3a9

Add apache license header.

a661690

Address review comments.

1517885

Fix checkstyle

62156d1

checkstyle

5909668

Checkstyle

da9c22c

rakeshadr reviewed Jan 5, 2022

View reviewed changes

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/BaseFileChecksumHelper.java Outdated Show resolved Hide resolved

rakeshadr reviewed Jan 5, 2022

View reviewed changes

jojochuang force-pushed the HDDS-6086 branch from b879459 to 5cce500 Compare January 5, 2022 08:22

rakeshadr reviewed Jan 5, 2022

View reviewed changes

hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java Outdated Show resolved Hide resolved

jojochuang force-pushed the HDDS-6086 branch from 859843f to da9c22c Compare January 7, 2022 05:27

jojochuang added 2 commits January 7, 2022 13:53

Validate the case where length <= 0

4395c5d

Fix checkstyle

3499d3d

lokeshj1703 reviewed Jan 7, 2022

View reviewed changes

Remove "hdfs"

d48a08e

rakeshadr approved these changes Jan 7, 2022

View reviewed changes

jojochuang merged commit 9644f83 into apache:master Jan 10, 2022

adoroszlai reviewed Jan 10, 2022

View reviewed changes

HDDS-6086. Compute MD5MD5CRC file checksum using chunk checksums from DataNodes #2919

HDDS-6086. Compute MD5MD5CRC file checksum using chunk checksums from DataNodes #2919

Uh oh!

Conversation

jojochuang commented Dec 13, 2021

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

rakeshadr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rakeshadr commented Dec 16, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rakeshadr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rakeshadr Jan 5, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jojochuang commented Jan 6, 2022

Uh oh!

jojochuang commented Jan 7, 2022

Uh oh!

lokeshj1703 left a comment

Choose a reason for hiding this comment

Uh oh!

lokeshj1703 Jan 7, 2022

Choose a reason for hiding this comment

Uh oh!

jojochuang Jan 7, 2022

Choose a reason for hiding this comment

Uh oh!

lokeshj1703 Jan 10, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rakeshadr commented Jan 7, 2022

Uh oh!

rakeshadr left a comment

Choose a reason for hiding this comment

Uh oh!

jojochuang commented Jan 10, 2022

Uh oh!

adoroszlai Jan 10, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants