svn commit: r1504676 - in /hbase/trunk/hbase-server/src: main/java/org/apache/hadoop/hbase/util/FSHDFSUtils.java test/java/org/apache/hadoop/hbase/util/TestFSHDFSUtils.java

[te...@apache.org]

Author: tedyu
Date: Thu Jul 18 21:48:18 2013
New Revision: 1504676

URL: http://svn.apache.org/r1504676
Log:
HBASE-8699 Parameter to DistributedFileSystem#isFileClosed should be of type Path (Ted Yu)


Modified:
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSHDFSUtils.java
    hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSHDFSUtils.java

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSHDFSUtils.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSHDFSUtils.java?rev=1504676&r1=1504675&r2=1504676&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSHDFSUtils.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSHDFSUtils.java Thu Jul 18 21:48:18 2013
@@ -95,9 +95,13 @@ public class FSHDFSUtils extends FSUtils
     // is dead.  We set it to 61 seconds, 1 second than the default READ_TIMEOUT in HDFS, the
     // default value for DFS_CLIENT_SOCKET_TIMEOUT_KEY.
     long subsequentPause = conf.getInt("hbase.lease.recovery.dfs.timeout", 61 * 1000);
+    
+    Method isFileClosedMeth = null;
+    // whether we need to look for isFileClosed method
+    boolean findIsFileClosedMeth = true;
     boolean recovered = false;
     // We break the loop if we succeed the lease recovery, timeout, or we throw an exception.
-    for (int nbAttempt = 0; true; nbAttempt++) {
+    for (int nbAttempt = 0; !recovered; nbAttempt++) {
       recovered = recoverLease(dfs, nbAttempt, p, startWaiting);
       if (recovered) break;
       checkIfCancelled(reporter);
@@ -113,7 +117,20 @@ public class FSHDFSUtils extends FSUtils
           while ((EnvironmentEdgeManager.currentTimeMillis() - localStartWaiting) <
               subsequentPause) {
             Thread.sleep(conf.getInt("hbase.lease.recovery.pause", 1000));
-            if (isFileClosed(dfs, p)) break;
+            if (findIsFileClosedMeth) {
+              try {
+                isFileClosedMeth = dfs.getClass().getMethod("isFileClosed",
+                  new Class[]{ Path.class });
+              } catch (NoSuchMethodException nsme) {
+                LOG.debug("isFileClosed not available");
+              } finally {
+                findIsFileClosedMeth = false;
+              }
+            }
+            if (isFileClosedMeth != null && isFileClosed(dfs, isFileClosedMeth, p)) {
+              recovered = true;
+              break;
+            }
             checkIfCancelled(reporter);
           }
         }
@@ -181,21 +198,17 @@ public class FSHDFSUtils extends FSUtils
   /**
    * Call HDFS-4525 isFileClosed if it is available.
    * @param dfs
+   * @param m
    * @param p
    * @return True if file is closed.
    */
-  boolean isFileClosed(final DistributedFileSystem dfs, final Path p) {
+  private boolean isFileClosed(final DistributedFileSystem dfs, final Method m, final Path p) {
     try {
-      Method m = dfs.getClass().getMethod("isFileClosed", new Class<?>[] {String.class});
-      return (Boolean) m.invoke(dfs, p.toString());
+      return (Boolean) m.invoke(dfs, p);
     } catch (SecurityException e) {
       LOG.warn("No access", e);
-    } catch (NoSuchMethodException e) {
-      if (LOG.isTraceEnabled()) {
-        LOG.trace("noSuchMethod isFileClosed (HDFS-4525); making do without");
-      }
     } catch (Exception e) {
-      LOG.warn("Failed invocation", e);
+      LOG.warn("Failed invocation for " + p.toString(), e);
     }
     return false;
   }
@@ -205,4 +218,4 @@ public class FSHDFSUtils extends FSUtils
     if (reporter == null) return;
     if (!reporter.progress()) throw new InterruptedIOException("Operation cancelled");
   }
-}
\ No newline at end of file
+}

Modified: hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSHDFSUtils.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSHDFSUtils.java?rev=1504676&r1=1504675&r2=1504676&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSHDFSUtils.java (original)
+++ hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSHDFSUtils.java Thu Jul 18 21:48:18 2013
@@ -88,10 +88,10 @@ public class TestFSHDFSUtils {
     // therefore return true if we are to break the loop.
     Mockito.when(dfs.recoverLease(FILE)).
       thenReturn(false).thenReturn(false).thenReturn(true);
-    Mockito.when(dfs.isFileClosed(FILE.toString())).thenReturn(true);
+    Mockito.when(dfs.isFileClosed(FILE)).thenReturn(true);
     assertTrue(this.fsHDFSUtils.recoverDFSFileLease(dfs, FILE, HTU.getConfiguration(), reporter));
-    Mockito.verify(dfs, Mockito.times(3)).recoverLease(FILE);
-    Mockito.verify(dfs, Mockito.times(1)).isFileClosed(FILE.toString());
+    Mockito.verify(dfs, Mockito.times(2)).recoverLease(FILE);
+    Mockito.verify(dfs, Mockito.times(1)).isFileClosed(FILE);
   }
 
   /**
@@ -102,8 +102,8 @@ public class TestFSHDFSUtils {
      * Close status of a file. Copied over from HDFS-4525
      * @return true if file is already closed
      **/
-    public boolean isFileClosed(String f) throws IOException{
+    public boolean isFileClosed(Path f) throws IOException{
       return false;
     }
   }
-}
\ No newline at end of file
+}