Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDFS-13056. Add support for a new COMPOSITE_CRC FileChecksum which is comparable between different block layouts and between striped/replicated files #344

Closed
wants to merge 30 commits into from

Conversation

dennishuo
Copy link

No description provided.

Previously it failed to convert ms to seconds and thus reports aggregate
throughput as 1/1000x actual numbers. By recommendation from reviewers,
such a calculation is not in-scope for TestDFSIO anyways, so remove it.
Also, make all the bytes-to-mb and milliseconds-to-seconds conversions
consistent in the reporting messages to help avoid this type of error
in the future.
Adds new file-level ChecksumCombineMode options settable through config and
lower-level BlockChecksumOptions to indicate block-checksum types supported by
both blockChecksum and blockGroupChecksum in DataTransferProtocol.

CRCs are composed such that they are agnostic to block/chunk/cell layout and
thus can be compared between replicated-files and striped-files of
different underlying blocksize, bytes-per-crc, and cellSize settings.

Does not alter default behavior, and doesn't touch the data-read or
data-write paths at all.
Minor optimization by starting multiplier at x^8 and fix the behavior of
composing a zero-length crcB.
…OSITE_CRC.

Update BlockChecksumHelper's CRC composition to use the same data buffer
used in MD5 case, and factor our shared logic from the
StripedBlockChecksumReconstructor into an abstract base class so that
reconstruction logic can be shared between MD5CRC and COMPOSITE_CRC.
Encapsulate all the CRC internals such as tracking the CRC polynomial,
precomputing the monomial, etc., into this class so taht BlockChecksumHelper
and FileChecksumHelper only need to interact with the clean interfaces
of CrcComposer.
Wire it in to BlockChecksumHelper and use CrcComposer to regenerate
striped composite CRCs for missing EC data blocks.
Extract hooks in TestFileChecksum to allow a subclass to share core
tests while modifying expectations of a subset of tests; add
TestFileChecksumCompositeCrc which extends TestFileChecksum to
apply the same test suite to COMPOSITE_CRC, and add a test case
for comparing two replicated files with different block sizes.
Test confirms that MD5CRC will yield different checksums
between replicated vs striped, and two replicated files with
different block sizes, while COMPOSITE_CRC yields the same
checksum for all cases.
Fix a bug in handling byte-array updates with nonzero offset.
Refactor to just use stripeLength with COMPOSITE_CRC, where non-striped
COMPOSITE_CRC is just an edge case where stripeLength is longer than the
data range.
…hecksum.

Additionally, fix up remaining TODOs; add wrappers for late-evaluating
hex format of CRCs to pass into debug statements and clean up logging
logic.
-Gate creation of some debug objects on LOG.isDebugEnabled()
-Expand InterfaceAudience of CrcUtil/CrcComposer to include Common, Yarn
-Split out the CRC reassembly logic in BlockGroupNonStripedChecksumComputer#compute to helper function
-Remove "throws IOException" from CrcComposer.digest
Fixes hadoop.tools.TestHdfsConfigFields
Due to HDFS-13191 the byte buffer size will not match getLength(),
and equals() compares the entire buffer regardless of getLength(),
so other filesystems would be unable to match the COMPOSITE-CRC
FileChecksum.
-Remove explicit InterfaceStability declarations on new classes
-Add checks for digester != null in StripedBlockChecksumReconstructor
-Add bounds checking in CrcUtil.readInt/writeInt
-Throw instead of return null in FileChecksumHelper.makeFinalResult when combineMode not recognized
-Add BlockChecksumType to debug message in FileChecksumHelper.tryDatanode
-Follow usual style of delegating to multi-arg constructor in BlockChecksumOptions
-Instead of building late-evaluated "Object" representations for debug purposes, make CrcUtil just provide toString methods and wrap the calls getting the strings inside isDebugEnabled() checks.
-Mark getFileChecksumWithCombineMode as LimitedPrivate
-Add TestCopyMapperCompositeCrc extending TestCopyMapper with differentiation of behaviors between the checksum options in terms of what kinds of file layouts are supported.
-Remove String.format from some LOG.debug statements
-Make ReplicatedFileChecksumComputer raise PathIOExceptions
-Switch TestCrcUtil and TestCrcComposer to use LambdaTestUtils.intercept instead of junit ExpectedException
-Add InterfaceStability.Unstable annotations to LimitedPrivate classes
-Remove unnecessary isDebugEnabled() checks in FileChecksumHelper
-Add global test timeouts for TestCrcUtil and TestCrcComposer
Remove unnecessary LimitedPrivate annotation in DFSClient.
Fix up some javadoc formatting.
@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 0 Docker mode activated.
-1 patch 12 #344 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.
Subsystem Report/Notes
GITHUB PR #344
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-344/1/console
versions git=2.7.4
Powered by Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 0 Docker mode activated.
-1 patch 18 #344 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.
Subsystem Report/Notes
GITHUB PR #344
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-344/2/console
versions git=2.7.4
Powered by Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 0 Docker mode activated.
-1 patch 13 #344 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.
Subsystem Report/Notes
GITHUB PR #344
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-344/3/console
versions git=2.7.4
Powered by Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 0 Docker mode activated.
-1 patch 10 #344 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.
Subsystem Report/Notes
GITHUB PR #344
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-344/4/console
versions git=2.17.1
Powered by Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

@jojochuang
Copy link
Contributor

Close the PR since it was committed.

@jojochuang jojochuang closed this Aug 8, 2019
shanthoosh pushed a commit to shanthoosh/hadoop that referenced this pull request Oct 15, 2019
…nges

You can find more details on the bug fixes and API changes [here](https://github.com/facebook/rocksdb/releases). I upgraded to 5.7.3 since 5.8.0 has a regression [KAFKA-6100](https://issues.apache.org/jira/browse/KAFKA-6100)

All of our tests passed locally. I will monitor travis for failures.

Author: Bharath Kumarasubramanian <bkumaras@linkedin.com>

Reviewers: Boris Shkolnik <boryas@gmail.com>

Closes apache#344 from bharathkk/master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants