You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by ha...@apache.org on 2011/01/10 20:01:37 UTC

svn commit: r1057313 - in /hadoop/common/branches/branch-0.20-append: ./ src/hdfs/org/apache/hadoop/hdfs/ src/hdfs/org/apache/hadoop/hdfs/protocol/ src/hdfs/org/apache/hadoop/hdfs/server/namenode/ src/test/org/apache/hadoop/hdfs/

Author: hairong
Date: Mon Jan 10 19:01:36 2011
New Revision: 1057313

URL: http://svn.apache.org/viewvc?rev=1057313&view=rev
Log:
HDFS-1554. New semantics for recoverLease. Contributed by Hairong Kuang.

Modified:
    hadoop/common/branches/branch-0.20-append/CHANGES.txt
    hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/DFSClient.java
    hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/DistributedFileSystem.java
    hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
    hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
    hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/server/namenode/NameNode.java
    hadoop/common/branches/branch-0.20-append/src/test/org/apache/hadoop/hdfs/TestDFSClientRetries.java
    hadoop/common/branches/branch-0.20-append/src/test/org/apache/hadoop/hdfs/TestLeaseRecovery2.java

Modified: hadoop/common/branches/branch-0.20-append/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-append/CHANGES.txt?rev=1057313&r1=1057312&r2=1057313&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.20-append/CHANGES.txt (original)
+++ hadoop/common/branches/branch-0.20-append/CHANGES.txt Mon Jan 10 19:01:36 2011
@@ -94,6 +94,8 @@ Release 0.20-append - Unreleased
     HDFS-1555. Disallow pipelien recovery if a file is already being
     lease recovered. (hairong)
 
+    HDFS-1554. New semantics for recoverLease. (hairong)
+
 Release 0.20.3 - Unreleased
 
   NEW FEATURES

Modified: hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/DFSClient.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/DFSClient.java?rev=1057313&r1=1057312&r2=1057313&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/DFSClient.java (original)
+++ hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/DFSClient.java Mon Jan 10 19:01:36 2011
@@ -507,13 +507,14 @@ public class DFSClient implements FSCons
   /**
    * Recover a file's lease
    * @param src a file's path
+   * @return true if the file is already closed
    * @throws IOException
    */
-  void recoverLease(String src) throws IOException {
+  boolean recoverLease(String src) throws IOException {
     checkOpen();
     
     try {
-      namenode.recoverLease(src, clientName);
+      return namenode.recoverLease(src, clientName);
     } catch (RemoteException re) {
       throw re.unwrapRemoteException(FileNotFoundException.class,
                                      AccessControlException.class);

Modified: hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/DistributedFileSystem.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/DistributedFileSystem.java?rev=1057313&r1=1057312&r2=1057313&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/DistributedFileSystem.java (original)
+++ hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/DistributedFileSystem.java Mon Jan 10 19:01:36 2011
@@ -180,12 +180,14 @@ public class DistributedFileSystem exten
   }
 
   /** 
-   * Trigger the lease reovery of a file
+   * Start the lease recovery of a file
+   *
    * @param f a file
+   * @return true if the file is already closed
    * @throws IOException if an error occurs
    */
-  public void recoverLease(Path f) throws IOException {
-    dfs.recoverLease(getPathName(f));
+  public boolean recoverLease(Path f) throws IOException {
+    return dfs.recoverLease(getPathName(f));
   }
 
   /** This optional operation is not yet supported. */

Modified: hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/protocol/ClientProtocol.java?rev=1057313&r1=1057312&r2=1057313&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/protocol/ClientProtocol.java (original)
+++ hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/protocol/ClientProtocol.java Mon Jan 10 19:01:36 2011
@@ -40,9 +40,9 @@ public interface ClientProtocol extends 
    * Compared to the previous version the following changes have been introduced:
    * (Only the latest change is reflected.
    * The log of historical changes can be retrieved from the svn).
-   * 42: Introduce a lightweight recoverLease RPC
+   * 43: recoverLease return if the file is closed or not
    */
-  public static final long versionID = 42L;
+  public static final long versionID = 43L;
   
   ///////////////////////////////////////
   // File contents
@@ -124,12 +124,14 @@ public interface ClientProtocol extends 
   public LocatedBlock append(String src, String clientName) throws IOException;
   
   /**
-   * Trigger lease recovery to happen
-   * @param src path of the file to trigger lease recovery
+   * Start lease recovery
+   * 
+   * @param src path of the file to start lease recovery
    * @param clientName name of the current client
+   * @return true if the file is already closed
    * @throws IOException
    */
-  public void recoverLease(String src, String clientName) throws IOException;
+  public boolean recoverLease(String src, String clientName) throws IOException;
 
   /**
    * Set replication for an existing file.

Modified: hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java?rev=1057313&r1=1057312&r2=1057313&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java (original)
+++ hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java Mon Jan 10 19:01:36 2011
@@ -1051,7 +1051,7 @@ public class FSNamesystem implements FSC
 
     try {
       INode myFile = dir.getFileINode(src);
-      recoverLeaseInternal(myFile, src, holder, clientMachine);
+      recoverLeaseInternal(myFile, src, holder, clientMachine, false);
 
       try {
         verifyReplication(src, replication, clientMachine);
@@ -1126,16 +1126,17 @@ public class FSNamesystem implements FSC
   }
 
   /**
-   * Trigger to recover lease;
-   * When the method returns successfully, the lease has been recovered and
-   * the file is closed.
+   * Recover lease;
+   * Immediately revoke the lease of the current lease holder and start lease
+   * recovery so that the file can be forced to be closed.
    * 
-   * @param src the path of the file to trigger release
+   * @param src the path of the file to start lease recovery
    * @param holder the lease holder's name
    * @param clientMachine the client machine's name
+   * @return true if the file is already closed
    * @throws IOException
    */
-  synchronized void recoverLease(String src, String holder, String clientMachine)
+  synchronized boolean recoverLease(String src, String holder, String clientMachine)
   throws IOException {
     if (isInSafeMode()) {
       throw new SafeModeException(
@@ -1150,15 +1151,19 @@ public class FSNamesystem implements FSC
       throw new FileNotFoundException("File not found " + src);
     }
 
+    if (!inode.isUnderConstruction()) {
+      return true;
+    }
     if (isPermissionEnabled) {
       checkPathAccess(src, FsAction.WRITE);
     }
 
-    recoverLeaseInternal(inode, src, holder, clientMachine);
+    recoverLeaseInternal(inode, src, holder, clientMachine, true);
+    return false;
   }
   
   private void recoverLeaseInternal(INode fileInode, 
-      String src, String holder, String clientMachine)
+      String src, String holder, String clientMachine, boolean force)
   throws IOException {
     if (fileInode != null && fileInode.isUnderConstruction()) {
       INodeFileUnderConstruction pendingFile = (INodeFileUnderConstruction) fileInode;
@@ -1171,7 +1176,7 @@ public class FSNamesystem implements FSC
       // We found the lease for this file. And surprisingly the original
       // holder is trying to recreate this file. This should never occur.
       //
-      if (lease != null) {
+      if (!force && lease != null) {
         Lease leaseFile = leaseManager.getLeaseByPath(src);
         if (leaseFile != null && leaseFile.equals(lease)) { 
           throw new AlreadyBeingCreatedException(
@@ -1190,21 +1195,29 @@ public class FSNamesystem implements FSC
                     " on client " + clientMachine + 
                     " because pendingCreates is non-null but no leases found.");
       }
-      //
-      // If the original holder has not renewed in the last SOFTLIMIT 
-      // period, then start lease recovery.
-      //
-      if (lease.expiredSoftLimit()) {
-        LOG.info("startFile: recover lease " + lease + ", src=" + src +
+      if (force) {
+        // close now: no need to wait for soft lease expiration and 
+        // close only the file src
+        LOG.info("recoverLease: recover lease " + lease + ", src=" + src +
                  " from client " + pendingFile.clientName);
-        internalReleaseLease(lease, src);
+        internalReleaseLeaseOne(lease, src);
+      } else {
+        //
+        // If the original holder has not renewed in the last SOFTLIMIT 
+        // period, then start lease recovery.
+        //
+        if (lease.expiredSoftLimit()) {
+          LOG.info("startFile: recover lease " + lease + ", src=" + src +
+              " from client " + pendingFile.clientName);
+          internalReleaseLease(lease, src);
+        }
+        throw new AlreadyBeingCreatedException(
+            "failed to create file " + src + " for " + holder +
+            " on client " + clientMachine + 
+            ", because this file is already being created by " +
+            pendingFile.getClientName() + 
+            " on " + pendingFile.getClientMachine());
       }
-      throw new AlreadyBeingCreatedException(
-                    "failed to create file " + src + " for " + holder +
-                    " on client " + clientMachine + 
-                    ", because this file is already being created by " +
-                    pendingFile.getClientName() + 
-                    " on " + pendingFile.getClientMachine());
     }
 
   }

Modified: hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/server/namenode/NameNode.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/server/namenode/NameNode.java?rev=1057313&r1=1057312&r2=1057313&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/server/namenode/NameNode.java (original)
+++ hadoop/common/branches/branch-0.20-append/src/hdfs/org/apache/hadoop/hdfs/server/namenode/NameNode.java Mon Jan 10 19:01:36 2011
@@ -399,9 +399,9 @@ public class NameNode implements ClientP
   }
 
   /** {@inheritDoc} */
-  public void recoverLease(String src, String clientName) throws IOException {
+  public boolean recoverLease(String src, String clientName) throws IOException {
     String clientMachine = getClientMachine();
-    namesystem.recoverLease(src, clientName, clientMachine);
+    return namesystem.recoverLease(src, clientName, clientMachine);
   }
   
   /** {@inheritDoc} */

Modified: hadoop/common/branches/branch-0.20-append/src/test/org/apache/hadoop/hdfs/TestDFSClientRetries.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-append/src/test/org/apache/hadoop/hdfs/TestDFSClientRetries.java?rev=1057313&r1=1057312&r2=1057313&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.20-append/src/test/org/apache/hadoop/hdfs/TestDFSClientRetries.java (original)
+++ hadoop/common/branches/branch-0.20-append/src/test/org/apache/hadoop/hdfs/TestDFSClientRetries.java Mon Jan 10 19:01:36 2011
@@ -226,7 +226,7 @@ public class TestDFSClientRetries extend
     public void setTimes(String src, long mtime, long atime) throws IOException {}
 
     @Override
-    public void recoverLease(String src, String clientName) throws IOException {}
+    public boolean recoverLease(String src, String clientName) throws IOException {return true;}
 
   }
   

Modified: hadoop/common/branches/branch-0.20-append/src/test/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-append/src/test/org/apache/hadoop/hdfs/TestLeaseRecovery2.java?rev=1057313&r1=1057312&r2=1057313&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.20-append/src/test/org/apache/hadoop/hdfs/TestLeaseRecovery2.java (original)
+++ hadoop/common/branches/branch-0.20-append/src/test/org/apache/hadoop/hdfs/TestLeaseRecovery2.java Mon Jan 10 19:01:36 2011
@@ -64,7 +64,7 @@ public class TestLeaseRecovery2 extends 
       //create a file
       DistributedFileSystem dfs = (DistributedFileSystem)cluster.getFileSystem();
       int size = AppendTestUtil.nextInt(FILE_SIZE);
-      Path filepath = createFile(dfs, size);
+      Path filepath = createFile(dfs, size, true);
 
       // set the soft limit to be 1 second so that the
       // namenode triggers lease recovery on next attempt to write-for-open.
@@ -74,59 +74,63 @@ public class TestLeaseRecovery2 extends 
       verifyFile(dfs, filepath, actual, size);
 
       //test recoverLease
+      // set the soft limit to be 1 hour but recoverLease should
+      // close the file immediately
+      cluster.setLeasePeriod(hardLease, hardLease);
       size = AppendTestUtil.nextInt(FILE_SIZE);
-      filepath = createFile(dfs, size);
+      filepath = createFile(dfs, size, false);
 
-      // set the soft limit to be 1 second so that the
-      // namenode triggers lease recovery on next attempt to write-for-open.
-      cluster.setLeasePeriod(softLease, hardLease);
+      // test recoverLese from a different client
+      recoverLease(filepath, null);
+      verifyFile(dfs, filepath, actual, size);
 
-      recoverLease(filepath);
+      // test recoverlease from the same client
+      size = AppendTestUtil.nextInt(FILE_SIZE);
+      filepath = createFile(dfs, size, false);
+      
+      // create another file using the same client
+      Path filepath1 = new Path("/foo" + AppendTestUtil.nextInt());
+      FSDataOutputStream stm = dfs.create(filepath1, true,
+          bufferSize, REPLICATION_NUM, BLOCK_SIZE);
+      
+      // recover the first file
+      recoverLease(filepath, dfs);
       verifyFile(dfs, filepath, actual, size);
 
+      // continue to write to the second file
+      stm.write(buffer, 0, size);
+      stm.close();
+      verifyFile(dfs, filepath1, actual, size);
     }
     finally {
       try {
-        if (cluster != null) {cluster.shutdown();}
+        if (cluster != null) {cluster.getFileSystem().close();cluster.shutdown();}
       } catch (Exception e) {
         // ignore
       }
     }
   }
   
-  private void recoverLease(Path filepath) throws IOException {
-    Configuration conf2 = new Configuration(conf);
-    String username = UserGroupInformation.getCurrentUGI().getUserName()+"_1";
-    UnixUserGroupInformation.saveToConf(conf2,
-        UnixUserGroupInformation.UGI_PROPERTY_NAME,
-        new UnixUserGroupInformation(username, new String[]{"supergroup"}));
-    DistributedFileSystem dfs2 = (DistributedFileSystem)FileSystem.get(conf2);
-
-    boolean done = false;
-    while (!done) {
-      try {
-        dfs2.recoverLease(filepath);
-        done = true;
-      } catch (IOException ioe) {
-        final String message = ioe.getMessage();
-        if (message.contains(AlreadyBeingCreatedException.class.getSimpleName())) {
-          AppendTestUtil.LOG.info("GOOD! got " + message);
-        }
-        else {
-          AppendTestUtil.LOG.warn("UNEXPECTED IOException", ioe);
-        }
-      }
-
-      if (!done) {
-        AppendTestUtil.LOG.info("sleep " + 1000 + "ms");
-        try {Thread.sleep(5000);} catch (InterruptedException e) {}
-      }
+  private void recoverLease(Path filepath, DistributedFileSystem dfs2) throws Exception {
+    if (dfs2==null) {
+      Configuration conf2 = new Configuration(conf);
+      String username = UserGroupInformation.getCurrentUGI().getUserName()+"_1";
+      UnixUserGroupInformation.saveToConf(conf2,
+          UnixUserGroupInformation.UGI_PROPERTY_NAME,
+          new UnixUserGroupInformation(username, new String[]{"supergroup"}));
+      dfs2 = (DistributedFileSystem)FileSystem.get(conf2);
+    }
+    
+    while (!dfs2.recoverLease(filepath)) {
+      AppendTestUtil.LOG.info("sleep " + 5000 + "ms");
+      Thread.sleep(5000);
     }
   }
   
   // try to re-open the file before closing the previous handle. This
   // should fail but will trigger lease recovery.
-  private Path createFile(DistributedFileSystem dfs, int size) throws IOException, InterruptedException {
+  private Path createFile(DistributedFileSystem dfs, int size,
+      boolean triggerSoftLease) throws IOException, InterruptedException {
     // create a random file name
     String filestr = "/foo" + AppendTestUtil.nextInt();
     System.out.println("filestr=" + filestr);
@@ -142,8 +146,10 @@ public class TestLeaseRecovery2 extends 
     // sync file
     AppendTestUtil.LOG.info("sync");
     stm.sync();
-    AppendTestUtil.LOG.info("leasechecker.interruptAndJoin()");
-    dfs.dfs.leasechecker.interruptAndJoin();
+    if (triggerSoftLease) {
+      AppendTestUtil.LOG.info("leasechecker.interruptAndJoin()");
+      dfs.dfs.leasechecker.interruptAndJoin();
+    }
     return filepath;
   }