HDDS-4138. Improve crc efficiency by using Java.util.zip.CRC when available #1950

sodonnel · 2021-02-22T12:31:45Z

What changes were proposed in this pull request?

Based on the results in HDDS-4808 (#1910 (comment)) it is clear we should switch the CRC32 implementations used by Ozone, as there are other better performing implementations.

Specifically:

org.apache.hadoop.ozone.common.PureJavaCrc32ByteBuffer to Java.util.zip.CRC32

org.apache.hadoop.ozone.common.PureJavaCrc32CByteBuffer to Java.util.zip.CRC32C if Java 9 or greater, otherwise org.apache.hadoop.util.PureJavaCrc32C

What is the link to the Apache JIRA

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

How was this patch tested?

Small change to an existing test

adoroszlai · 2021-02-22T18:20:32Z

@sodonnel hadoop27 test is failing due to:

NoSuchMethodError: org.apache.hadoop.util.Shell.isJavaVersionAtLeast(I)Z
	at org.apache.hadoop.ozone.common.ChecksumByteBufferImpl.<clinit>(ChecksumByteBufferImpl.java:38)

sodonnel · 2021-02-22T20:49:51Z

@adoroszlai Thanks for checking it. Is this because the Hadoop27 tests runs with a hadoop-common 2.x which I guess does not have the Shell.isJavaVersionAtLeast method I am using? I will figure out a way around this, perhaps just creating a method to the Java check, rather than reusing what is in Hadoop common.

… HDDS-4138-crc

sodonnel · 2021-02-22T20:58:05Z

For now (to ensure it fixes the test) I lifted the methods out of hadoop-common into the ChecksumByteBufferImpl class. However we should probably move them into a Ozone util class somewhere, so they are more discoverable / reusable.

adoroszlai · 2021-02-23T17:26:02Z

However we should probably move them into a Ozone util class somewhere, so they are more discoverable / reusable.

Totally agree. Maybe JavaUtils in hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds. (Normally I'd suggest keeping the class name from Hadoop in case we might need to copy more code later. But the Shell class itself does not seem to be very much related to the Java version check, so I think we can do better.)

swagle · 2021-02-24T02:04:23Z

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/ChecksumByteBufferImpl.java

+  public static ChecksumByteBuffer crc32Cimpl() {
+    if (useJava9Crc32C) {
+      try {
+        return new ChecksumByteBufferImpl(Java9Crc32CFactory.createChecksum());


Should we consider a static factory here for creating the ChecksumByteBufferImpl?

I'm not sure what you mean - the crc32Cimpl is like a static factory I think, and the code which needs to get a checksum impl calls either checksumByteBufferImpl.crc32impl or checksumByteBufferImpl..crc32cimpl`.

Can you post a code snippet of what you are thinking?

What I meant was to separate out the Factory class from the interface impl to encapsulate the object creation logic separate from the behavior that the impl provides, this suggestion was based on the fact that crc32Cimpl is like a static factory method but it is co-located with the impl.

sodonnel · 2021-02-25T11:12:03Z

@adoroszlai Have you any further comments or am I good to commit this?

…ing-upgrade * upstream/master: (29 commits) HDDS-4741. Modularize upgrade test (apache#1928) HDDS-4864. Add acceptance tests to certify Ozone with boto3 python client. (apache#1976) HDDS-4791. StateContext.getReports may return list with size larger t… (apache#1892) HDDS-4867. Ozone admin datanode list should report dead and stale nodes (apache#1966) HDDS-4858. Useless Maven cache cleanup (apache#1956) HDDS-4769. Simplify insert operation of ContainerAttribute (apache#1865) HDDS-4847. Fix typo in name of IdentityService (apache#1941) HDDS-4869. Bump jackson version number (apache#1963) HDDS-4871. Fix intellij runConfigurations for datanode (apache#1968) HDDS-4870. Bump jetty version (apache#1964) HDDS-4722. Creating RDBStore fails due to RDBMetrics instance race (apache#1820) HDDS-4138. Improve crc efficiency by using Java.util.zip.CRC when available (apache#1950) HDDS-4816. Add UsageInfoSubcommand to get Datanode usage information. (apache#1919) HDDS-4754. Make scm heartbeat rpc retry interval configurable (apache#1942) HDDS-4832. Show Datanode OperationalState in Recon (apache#1937) HDDS-4653. Support TDE for MPU Keys on Encrypted Buckets (apache#1766) HDDS-4853. libexec/entrypoint.sh might copy from wrong path (apache#1951) HDDS-4857. Format ReplicationType.java which indentation are confusion (apache#1952) HDDS-4850. Intermittent failure in ozonesecure due to unable to allocate block (apache#1948) HDDS-4808. Add Genesis benchmark for various CRC implementations (apache#1910) ... Conflicts: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/client/ScmClient.java hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java hadoop-hdds/interface-admin/src/main/proto/ScmAdminProtocol.proto hadoop-hdds/interface-client/src/main/proto/hdds.proto hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java

…ing-upgrade-merge-candidate * upstream/master: (29 commits) HDDS-4741. Modularize upgrade test (apache#1928) HDDS-4864. Add acceptance tests to certify Ozone with boto3 python client. (apache#1976) HDDS-4791. StateContext.getReports may return list with size larger t… (apache#1892) HDDS-4867. Ozone admin datanode list should report dead and stale nodes (apache#1966) HDDS-4858. Useless Maven cache cleanup (apache#1956) HDDS-4769. Simplify insert operation of ContainerAttribute (apache#1865) HDDS-4847. Fix typo in name of IdentityService (apache#1941) HDDS-4869. Bump jackson version number (apache#1963) HDDS-4871. Fix intellij runConfigurations for datanode (apache#1968) HDDS-4870. Bump jetty version (apache#1964) HDDS-4722. Creating RDBStore fails due to RDBMetrics instance race (apache#1820) HDDS-4138. Improve crc efficiency by using Java.util.zip.CRC when available (apache#1950) HDDS-4816. Add UsageInfoSubcommand to get Datanode usage information. (apache#1919) HDDS-4754. Make scm heartbeat rpc retry interval configurable (apache#1942) HDDS-4832. Show Datanode OperationalState in Recon (apache#1937) HDDS-4653. Support TDE for MPU Keys on Encrypted Buckets (apache#1766) HDDS-4853. libexec/entrypoint.sh might copy from wrong path (apache#1951) HDDS-4857. Format ReplicationType.java which indentation are confusion (apache#1952) HDDS-4850. Intermittent failure in ozonesecure due to unable to allocate block (apache#1948) HDDS-4808. Add Genesis benchmark for various CRC implementations (apache#1910) ... Conflicts: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/client/ScmClient.java hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java hadoop-hdds/interface-admin/src/main/proto/ScmAdminProtocol.proto hadoop-hdds/interface-client/src/main/proto/hdds.proto hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconNodeManager.java

* HDDS-3698-nonrolling-upgrade: (29 commits) HDDS-4741. Modularize upgrade test (apache#1928) HDDS-4864. Add acceptance tests to certify Ozone with boto3 python client. (apache#1976) HDDS-4791. StateContext.getReports may return list with size larger t… (apache#1892) HDDS-4867. Ozone admin datanode list should report dead and stale nodes (apache#1966) HDDS-4858. Useless Maven cache cleanup (apache#1956) HDDS-4769. Simplify insert operation of ContainerAttribute (apache#1865) HDDS-4847. Fix typo in name of IdentityService (apache#1941) HDDS-4869. Bump jackson version number (apache#1963) HDDS-4871. Fix intellij runConfigurations for datanode (apache#1968) HDDS-4870. Bump jetty version (apache#1964) HDDS-4722. Creating RDBStore fails due to RDBMetrics instance race (apache#1820) HDDS-4138. Improve crc efficiency by using Java.util.zip.CRC when available (apache#1950) HDDS-4816. Add UsageInfoSubcommand to get Datanode usage information. (apache#1919) HDDS-4754. Make scm heartbeat rpc retry interval configurable (apache#1942) HDDS-4832. Show Datanode OperationalState in Recon (apache#1937) HDDS-4653. Support TDE for MPU Keys on Encrypted Buckets (apache#1766) HDDS-4853. libexec/entrypoint.sh might copy from wrong path (apache#1951) HDDS-4857. Format ReplicationType.java which indentation are confusion (apache#1952) HDDS-4850. Intermittent failure in ozonesecure due to unable to allocate block (apache#1948) HDDS-4808. Add Genesis benchmark for various CRC implementations (apache#1910) ...

S O'Donnell and others added 2 commits February 22, 2021 12:22

Initial changes

42ae133

trigger new CI check

cbd638f

S O'Donnell added 2 commits February 22, 2021 20:55

Lift methods to check Java version from hadoop-common

21ffacb

Merge branch 'HDDS-4138-crc' of github.com:sodonnel/hadoop-ozone into…

b43945f

… HDDS-4138-crc

swagle reviewed Feb 24, 2021

View reviewed changes

S O'Donnell added 3 commits February 24, 2021 11:32

Moved Java version methods into a Util class

8b08582

move factory methods into a factory class

12df41f

Added missing factory class

16015c7

swagle approved these changes Feb 24, 2021

View reviewed changes

adoroszlai approved these changes Feb 25, 2021

View reviewed changes

sodonnel merged commit 85e27c3 into apache:master Feb 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HDDS-4138. Improve crc efficiency by using Java.util.zip.CRC when available #1950

HDDS-4138. Improve crc efficiency by using Java.util.zip.CRC when available #1950

Uh oh!

sodonnel commented Feb 22, 2021

Uh oh!

adoroszlai commented Feb 22, 2021

Uh oh!

sodonnel commented Feb 22, 2021

Uh oh!

sodonnel commented Feb 22, 2021

Uh oh!

adoroszlai commented Feb 23, 2021

Uh oh!

swagle Feb 24, 2021

Uh oh!

sodonnel Feb 24, 2021

Uh oh!

swagle Feb 24, 2021

Uh oh!

sodonnel commented Feb 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HDDS-4138. Improve crc efficiency by using Java.util.zip.CRC when available #1950

HDDS-4138. Improve crc efficiency by using Java.util.zip.CRC when available #1950

Uh oh!

Conversation

sodonnel commented Feb 22, 2021

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

adoroszlai commented Feb 22, 2021

Uh oh!

sodonnel commented Feb 22, 2021

Uh oh!

sodonnel commented Feb 22, 2021

Uh oh!

adoroszlai commented Feb 23, 2021

Uh oh!

swagle Feb 24, 2021

Choose a reason for hiding this comment

Uh oh!

sodonnel Feb 24, 2021

Choose a reason for hiding this comment

Uh oh!

swagle Feb 24, 2021

Choose a reason for hiding this comment

Uh oh!

sodonnel commented Feb 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants