Skip to content

HADOOP-18203. Backport HADOOP-15033 to branch-2.10. #4174

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

Open
wants to merge 3 commits into
base: branch-2.10
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.fs.ChecksumException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;

/**
* This class provides interface and utilities for processing checksums for
Expand All @@ -43,6 +49,9 @@ public class DataChecksum implements Checksum {
public static final int CHECKSUM_CRC32C = 2;
public static final int CHECKSUM_DEFAULT = 3;
public static final int CHECKSUM_MIXED = 4;

private static final Logger LOG = LoggerFactory.getLogger(DataChecksum.class);
private static volatile boolean useJava9Crc32C = Shell.isJavaVersionAtLeast(9);

/** The checksum types */
public static enum Type {
Expand Down Expand Up @@ -78,6 +87,23 @@ public static Checksum newCrc32() {
return new CRC32();
}


/**
* The flag is volatile to avoid synchronization here.
* Re-entrancy is unlikely except in failure mode (and inexpensive).
*/
static Checksum newCrc32C() {
try {
return useJava9Crc32C ? Java9Crc32CFactory.createChecksum()
: new PureJavaCrc32C();
} catch (ExceptionInInitializerError | RuntimeException e) {
// should not happen
LOG.error("CRC32C creation failed, switching to PureJavaCrc32C", e);
useJava9Crc32C = false;
return new PureJavaCrc32C();
}
}

public static DataChecksum newDataChecksum(Type type, int bytesPerChecksum ) {
if ( bytesPerChecksum <= 0 ) {
return null;
Expand All @@ -89,7 +115,7 @@ public static DataChecksum newDataChecksum(Type type, int bytesPerChecksum ) {
case CRC32 :
return new DataChecksum(type, newCrc32(), bytesPerChecksum );
case CRC32C:
return new DataChecksum(type, new PureJavaCrc32C(), bytesPerChecksum);
return new DataChecksum(type, newCrc32C(), bytesPerChecksum);
default:
return null;
}
Expand Down Expand Up @@ -520,4 +546,36 @@ public void update(byte[] b, int off, int len) {}
@Override
public void update(int b) {}
};

/**
* Holds constructor handle to let it be initialized on demand.
*/
private static class Java9Crc32CFactory {
private static final MethodHandle NEW_CRC32C_MH;

static {
MethodHandle newCRC32C = null;
try {
newCRC32C = MethodHandles.publicLookup()
.findConstructor(
Class.forName("java.util.zip.CRC32C"),
MethodType.methodType(void.class)
);
} catch (ReflectiveOperationException e) {
// Should not reach here.
throw new RuntimeException(e);
}
NEW_CRC32C_MH = newCRC32C;
}

public static Checksum createChecksum() {
try {
// Should throw nothing
return (Checksum) NEW_CRC32C_MH.invoke();
} catch (Throwable t) {
throw (t instanceof RuntimeException) ? (RuntimeException) t
: new RuntimeException(t);
}
}
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,21 @@ public static boolean isJava7OrAbove() {
return true;
}

// "1.8"->8, "9"->9, "10"->10
private static final int JAVA_SPEC_VER = Math.max(8, Integer.parseInt(
System.getProperty("java.specification.version").split("\\.")[0]));
Comment on lines +93 to +94
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JAVA_SPEC_VER is set to 8 even on Java 7. Can we cover Java 7 here? If not, we should have a comment for justification at least.


/**
* Query to see if major version of Java specification of the system
* is equal or greater than the parameter.
*
* @param version 8, 9, 10 etc.
* @return comparison with system property, always true for 8
*/
public static boolean isJavaVersionAtLeast(int version) {
return JAVA_SPEC_VER >= version;
}

/**
* Maximum command line length in Windows
* KB830473 documents this as 8191
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,18 @@ public DataChecksum.Type crcType() {
}
}

final class ZipC extends AbstractCrc32<Checksum> {
@Override
public Checksum newAlgorithm() {
return DataChecksum.newCrc32C();
}

@Override
public DataChecksum.Type crcType() {
return DataChecksum.Type.CRC32C;
}
}

final class PureJava extends AbstractCrc32<PureJavaCrc32> {
@Override
public PureJavaCrc32 newAlgorithm() {
Expand Down Expand Up @@ -169,6 +181,9 @@ public DataChecksum.Type crcType() {
this.direct = direct;

crcs.add(Crc32.Zip.class);
if (Shell.isJavaVersionAtLeast(9)) {
crcs.add(Crc32.ZipC.class);
}
crcs.add(Crc32.PureJava.class);
crcs.add(Crc32.PureJavaC.class);

Expand Down Expand Up @@ -435,4 +450,4 @@ static void printSystemProperties(PrintStream outCrc) {
outCrc.printf("%" + max + "s = %s\n", n, p.getProperty(n));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -528,4 +528,9 @@ public Boolean get() {
shexc1.getProcess().waitFor();
shexc2.getProcess().waitFor();
}

@Test
public void testIsJavaVersionAtLeast() {
assertTrue(Shell.isJavaVersionAtLeast(8));
}
Comment on lines +533 to +535
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test succeeds on Java 7 since the Shell.isJavaVersionAtLeast(8) returns true even on Java 7. We should have a comment for justification at least.

}
1 change: 1 addition & 0 deletions hadoop-project/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1599,6 +1599,7 @@
<ignore>sun.nio.ch.*</ignore>
<ignore>com.sun.javadoc.*</ignore>
<ignore>com.sun.tools.*</ignore>
<ignore>java.lang.invoke.*</ignore>
</ignores>
</configuration>
</plugin>
Expand Down