stoty commented on code in PR #6342:
URL: https://github.com/apache/hbase/pull/6342#discussion_r1802608125
##########
hbase-asyncfs/src/main/java/org/apache/hadoop/hbase/util/RecoverLeaseFSUtils.java:
##########
@@ -39,6 +41,36 @@ public final class RecoverLeaseFSUtils {
private static final Logger LOG =
LoggerFactory.getLogger(RecoverLeaseFSUtils.class);
+ private static Class<?> leaseRecoverableClazz = null;
+ private static Method recoverLeaseMethod = null;
+ public static final String LEASE_RECOVERABLE_CLASS_NAME =
"org.apache.hadoop.fs.LeaseRecoverable";
+ static {
+ LOG.debug("RecoverLeaseFSUtils loaded");
Review Comment:
nit: This should be after initializeRecoverLeaseMethod
##########
hbase-asyncfs/src/main/java/org/apache/hadoop/hbase/util/RecoverLeaseFSUtils.java:
##########
@@ -39,6 +41,36 @@ public final class RecoverLeaseFSUtils {
private static final Logger LOG =
LoggerFactory.getLogger(RecoverLeaseFSUtils.class);
+ private static Class<?> leaseRecoverableClazz = null;
+ private static Method recoverLeaseMethod = null;
+ public static final String LEASE_RECOVERABLE_CLASS_NAME =
"org.apache.hadoop.fs.LeaseRecoverable";
+ static {
+ LOG.debug("RecoverLeaseFSUtils loaded");
+ initializeRecoverLeaseMethod(LEASE_RECOVERABLE_CLASS_NAME);
+ }
+
+ /**
+ * Initialize reflection classes and methods. If LeaseRecoverable class is
not found,
+ * look for DistributedFilSystem#recoverLease method.
+ */
+ static void initializeRecoverLeaseMethod(String className) {
+ try {
+ leaseRecoverableClazz = Class.forName(className);
+ recoverLeaseMethod = leaseRecoverableClazz.getMethod("recoverLease",
Path.class);
+ LOG.debug("set recoverLeaseMethod to " + className + ".recoverLease()");
+ } catch (ClassNotFoundException e) {
+ LOG.debug(
+ "LeaseRecoverable interface not in the classpath, this means Hadoop
3.3.5 or below.");
+ try {
+ recoverLeaseMethod =
DistributedFileSystem.class.getMethod("recoverLease", Path.class);
Review Comment:
We expect to have this on all supported releases, right ?
##########
hbase-asyncfs/src/main/java/org/apache/hadoop/hbase/util/RecoverLeaseFSUtils.java:
##########
@@ -39,6 +41,36 @@ public final class RecoverLeaseFSUtils {
private static final Logger LOG =
LoggerFactory.getLogger(RecoverLeaseFSUtils.class);
+ private static Class<?> leaseRecoverableClazz = null;
+ private static Method recoverLeaseMethod = null;
+ public static final String LEASE_RECOVERABLE_CLASS_NAME =
"org.apache.hadoop.fs.LeaseRecoverable";
+ static {
+ LOG.debug("RecoverLeaseFSUtils loaded");
+ initializeRecoverLeaseMethod(LEASE_RECOVERABLE_CLASS_NAME);
+ }
+
+ /**
+ * Initialize reflection classes and methods. If LeaseRecoverable class is
not found,
+ * look for DistributedFilSystem#recoverLease method.
+ */
+ static void initializeRecoverLeaseMethod(String className) {
+ try {
+ leaseRecoverableClazz = Class.forName(className);
+ recoverLeaseMethod = leaseRecoverableClazz.getMethod("recoverLease",
Path.class);
+ LOG.debug("set recoverLeaseMethod to " + className + ".recoverLease()");
+ } catch (ClassNotFoundException e) {
+ LOG.debug(
+ "LeaseRecoverable interface not in the classpath, this means Hadoop
3.3.5 or below.");
+ try {
+ recoverLeaseMethod =
DistributedFileSystem.class.getMethod("recoverLease", Path.class);
Review Comment:
Maybe log an error message to explain that this should never happen ?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java:
##########
@@ -247,8 +262,19 @@ public static void checkFileSystemAvailable(final
FileSystem fs) throws IOExcept
* @param dfs A DistributedFileSystem object representing the underlying
HDFS.
* @return whether we're in safe mode
*/
- private static boolean isInSafeMode(DistributedFileSystem dfs) throws
IOException {
- return dfs.setSafeMode(SAFEMODE_GET, true);
+ private static boolean isInSafeMode(FileSystem dfs) throws IOException {
+ if (isDistributedFileSystem(dfs)) {
+ return ((DistributedFileSystem) dfs).setSafeMode(SAFEMODE_GET, true);
+ } else {
+ try {
+ Object ret = dfs.getClass()
+ .getMethod("setSafeMode", new Class[] { safeModeActionClazz,
Boolean.class })
+ .invoke(dfs, safeModeGet, true);
+ return (Boolean) ret;
+ } catch (InvocationTargetException | IllegalAccessException |
NoSuchMethodException e) {
+ throw new RuntimeException(e);
Review Comment:
Maybe log an error message to explain that this should never happen ?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java:
##########
@@ -247,8 +262,19 @@ public static void checkFileSystemAvailable(final
FileSystem fs) throws IOExcept
* @param dfs A DistributedFileSystem object representing the underlying
HDFS.
* @return whether we're in safe mode
*/
- private static boolean isInSafeMode(DistributedFileSystem dfs) throws
IOException {
- return dfs.setSafeMode(SAFEMODE_GET, true);
+ private static boolean isInSafeMode(FileSystem dfs) throws IOException {
+ if (isDistributedFileSystem(dfs)) {
+ return ((DistributedFileSystem) dfs).setSafeMode(SAFEMODE_GET, true);
+ } else {
+ try {
+ Object ret = dfs.getClass()
+ .getMethod("setSafeMode", new Class[] { safeModeActionClazz,
Boolean.class })
+ .invoke(dfs, safeModeGet, true);
+ return (Boolean) ret;
+ } catch (InvocationTargetException | IllegalAccessException |
NoSuchMethodException e) {
+ throw new RuntimeException(e);
Review Comment:
There is no Hadoop release where we expect to get here, right ?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java:
##########
@@ -247,8 +262,19 @@ public static void checkFileSystemAvailable(final
FileSystem fs) throws IOExcept
* @param dfs A DistributedFileSystem object representing the underlying
HDFS.
* @return whether we're in safe mode
*/
- private static boolean isInSafeMode(DistributedFileSystem dfs) throws
IOException {
- return dfs.setSafeMode(SAFEMODE_GET, true);
+ private static boolean isInSafeMode(FileSystem dfs) throws IOException {
+ if (isDistributedFileSystem(dfs)) {
+ return ((DistributedFileSystem) dfs).setSafeMode(SAFEMODE_GET, true);
+ } else {
+ try {
Review Comment:
Should we just return false here if we could not find the method previously ?
i.e. if this is an older Hadoop, and this is called with say,
LocalFilesystem ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
