Skip to content

HBASE-23686 Revert binary incompatible change in ByteRangeUtils and removed reflections in CommonFSUtils #1073

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.

Sign up for GitHub

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

Merged
HorizonNet merged 1 commit into from
Jan 24, 2020
Merged

HBASE-23686 Revert binary incompatible change in ByteRangeUtils and removed reflections in CommonFSUtils #1073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions hbase-checkstyle/src/main/resources/hbase/checkstyle-suppressions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,8 @@
<suppress checks="EmptyBlock" files="org.apache.hadoop.hbase.TestTimeout"/>
<suppress checks="InnerAssignment" files="org.apache.hadoop.hbase.rest.PerformanceEvaluation"/>
<suppress checks="EmptyBlock" files="org.apache.hadoop.hbase.rest.PerformanceEvaluation"/>
<!-- Will not have a private constructor, because it is InterfaceAudience.Public -->
<suppress checks="HideUtilityClassConstructor" files="org.apache.hadoop.hbase.util.ByteRangeUtils"/>
<!-- Will not be final, because it is InterfaceAudience.Public -->
<suppress checks="FinalClass" files="org.apache.hadoop.hbase.net.Address"/>
</suppressions>
2 changes: 1 addition & 1 deletion hbase-common/src/main/java/org/apache/hadoop/hbase/net/Address.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
* We cannot have Guava classes in our API hence this Type.
*/
@InterfaceAudience.Public
public final class Address implements Comparable<Address> {
public class Address implements Comparable<Address> {
private HostAndPort hostAndPort;

private Address(HostAndPort hostAndPort) {
Expand Down
5 changes: 1 addition & 4 deletions hbase-common/src/main/java/org/apache/hadoop/hbase/util/ByteRangeUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@
* Utility methods for working with {@link ByteRange}.
*/
@InterfaceAudience.Public
public final class ByteRangeUtils {
private ByteRangeUtils() {
}

public class ByteRangeUtils {
public static int numEqualPrefixBytes(ByteRange left, ByteRange right, int rightInnerOffset) {
int maxCompares = Math.min(left.getLength(), right.getLength() - rightInnerOffset);
final byte[] lbytes = left.getBytes();
Expand Down
152 changes: 37 additions & 115 deletions hbase-common/src/main/java/org/apache/hadoop/hbase/util/CommonFSUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,69 +148,22 @@ public static boolean deleteDirectory(final FileSystem fs, final Path dir) throw
* Return the number of bytes that large input files should be optimally
* be split into to minimize i/o time.
*
* use reflection to search for getDefaultBlockSize(Path f)
* if the method doesn't exist, fall back to using getDefaultBlockSize()
*
* @param fs filesystem object
* @return the default block size for the path's filesystem
* @throws IOException e
*/
public static long getDefaultBlockSize(final FileSystem fs, final Path path) throws IOException {
Method m = null;
Class<? extends FileSystem> cls = fs.getClass();
try {
m = cls.getMethod("getDefaultBlockSize", Path.class);
} catch (NoSuchMethodException e) {
LOG.info("FileSystem doesn't support getDefaultBlockSize");
} catch (SecurityException e) {
LOG.info("Doesn't have access to getDefaultBlockSize on FileSystems", e);
m = null; // could happen on setAccessible()
}
if (m == null) {
return fs.getDefaultBlockSize(path);
} else {
try {
Object ret = m.invoke(fs, path);
return ((Long)ret).longValue();
} catch (Exception e) {
throw new IOException(e);
}
}
public static long getDefaultBlockSize(final FileSystem fs, final Path path) {
return fs.getDefaultBlockSize(path);
}

/*
* Get the default replication.
*
* use reflection to search for getDefaultReplication(Path f)
* if the method doesn't exist, fall back to using getDefaultReplication()
*
* @param fs filesystem object
* @param f path of file
* @return default replication for the path's filesystem
* @throws IOException e
*/
public static short getDefaultReplication(final FileSystem fs, final Path path)
throws IOException {
Method m = null;
Class<? extends FileSystem> cls = fs.getClass();
try {
m = cls.getMethod("getDefaultReplication", Path.class);
} catch (NoSuchMethodException e) {
LOG.info("FileSystem doesn't support getDefaultReplication");
} catch (SecurityException e) {
LOG.info("Doesn't have access to getDefaultReplication on FileSystems", e);
m = null; // could happen on setAccessible()
}
if (m == null) {
return fs.getDefaultReplication(path);
} else {
try {
Object ret = m.invoke(fs, path);
return ((Number)ret).shortValue();
} catch (Exception e) {
throw new IOException(e);
}
}
public static short getDefaultReplication(final FileSystem fs, final Path path) {
return fs.getDefaultReplication(path);
}

/**
Expand Down Expand Up @@ -568,83 +521,52 @@ static void setStoragePolicy(final FileSystem fs, final Path path, final String
*/
private static void invokeSetStoragePolicy(final FileSystem fs, final Path path,
final String storagePolicy) throws IOException {
Method m = null;
Exception toThrow = null;

try {
m = fs.getClass().getDeclaredMethod("setStoragePolicy", Path.class, String.class);
m.setAccessible(true);
} catch (NoSuchMethodException e) {
toThrow = e;
final String msg = "FileSystem doesn't support setStoragePolicy; HDFS-6584, HDFS-9345 " +
"not available. This is normal and expected on earlier Hadoop versions.";
if (!warningMap.containsKey(fs)) {
warningMap.put(fs, true);
LOG.warn(msg, e);
} else if (LOG.isDebugEnabled()) {
LOG.debug(msg, e);
fs.setStoragePolicy(path, storagePolicy);

if (LOG.isDebugEnabled()) {
LOG.debug("Set storagePolicy={} for path={}", storagePolicy, path);
}
m = null;
} catch (SecurityException e) {
} catch (Exception e) {
toThrow = e;
final String msg = "No access to setStoragePolicy on FileSystem from the SecurityManager; " +
"HDFS-6584, HDFS-9345 not available. This is unusual and probably warrants an email " +
"to the user@hbase mailing list. Please be sure to include a link to your configs, and " +
"logs that include this message and period of time before it. Logs around service " +
"start up will probably be useful as well.";
// This swallows FNFE, should we be throwing it? seems more likely to indicate dev
// misuse than a runtime problem with HDFS.
if (!warningMap.containsKey(fs)) {
warningMap.put(fs, true);
LOG.warn(msg, e);
LOG.warn("Unable to set storagePolicy=" + storagePolicy + " for path=" + path + ". " +
"DEBUG log level might have more details.", e);
} else if (LOG.isDebugEnabled()) {
LOG.debug(msg, e);
LOG.debug("Unable to set storagePolicy=" + storagePolicy + " for path=" + path, e);
}
m = null; // could happen on setAccessible() or getDeclaredMethod()
}
if (m != null) {
try {
m.invoke(fs, path, storagePolicy);

// check for lack of HDFS-7228
if (e instanceof RemoteException &&
HadoopIllegalArgumentException.class.getName().equals(
((RemoteException)e).getClassName())) {
if (LOG.isDebugEnabled()) {
LOG.debug("Set storagePolicy={} for path={}", storagePolicy, path);
}
} catch (Exception e) {
toThrow = e;
// This swallows FNFE, should we be throwing it? seems more likely to indicate dev
// misuse than a runtime problem with HDFS.
if (!warningMap.containsKey(fs)) {
warningMap.put(fs, true);
LOG.warn("Unable to set storagePolicy=" + storagePolicy + " for path=" + path + ". " +
"DEBUG log level might have more details.", e);
} else if (LOG.isDebugEnabled()) {
LOG.debug("Unable to set storagePolicy=" + storagePolicy + " for path=" + path, e);
LOG.debug("Given storage policy, '" +storagePolicy +"', was rejected and probably " +
"isn't a valid policy for the version of Hadoop you're running. I.e. if you're " +
"trying to use SSD related policies then you're likely missing HDFS-7228. For " +
"more information see the 'ArchivalStorage' docs for your Hadoop release.");
}
// check for lack of HDFS-7228
if (e instanceof InvocationTargetException) {
final Throwable exception = e.getCause();
if (exception instanceof RemoteException &&
HadoopIllegalArgumentException.class.getName().equals(
((RemoteException)exception).getClassName())) {
if (LOG.isDebugEnabled()) {
LOG.debug("Given storage policy, '" +storagePolicy +"', was rejected and probably " +
"isn't a valid policy for the version of Hadoop you're running. I.e. if you're " +
"trying to use SSD related policies then you're likely missing HDFS-7228. For " +
"more information see the 'ArchivalStorage' docs for your Hadoop release.");
}
// Hadoop 2.8+, 3.0-a1+ added FileSystem.setStoragePolicy with a default implementation
// that throws UnsupportedOperationException
} else if (exception instanceof UnsupportedOperationException) {
if (LOG.isDebugEnabled()) {
LOG.debug("The underlying FileSystem implementation doesn't support " +
"setStoragePolicy. This is probably intentional on their part, since HDFS-9345 " +
"appears to be present in your version of Hadoop. For more information check " +
"the Hadoop documentation on 'ArchivalStorage', the Hadoop FileSystem " +
"specification docs from HADOOP-11981, and/or related documentation from the " +
"provider of the underlying FileSystem (its name should appear in the " +
"stacktrace that accompanies this message). Note in particular that Hadoop's " +
"local filesystem implementation doesn't support storage policies.", exception);
}
}
// Hadoop 2.8+, 3.0-a1+ added FileSystem.setStoragePolicy with a default implementation
// that throws UnsupportedOperationException
} else if (e instanceof UnsupportedOperationException) {
if (LOG.isDebugEnabled()) {
LOG.debug("The underlying FileSystem implementation doesn't support " +
"setStoragePolicy. This is probably intentional on their part, since HDFS-9345 " +
"appears to be present in your version of Hadoop. For more information check " +
"the Hadoop documentation on 'ArchivalStorage', the Hadoop FileSystem " +
"specification docs from HADOOP-11981, and/or related documentation from the " +
"provider of the underlying FileSystem (its name should appear in the " +
"stacktrace that accompanies this message). Note in particular that Hadoop's " +
"local filesystem implementation doesn't support storage policies.", e);
}
}
}

if (toThrow != null) {
throw new IOException(toThrow);
}
Expand Down