HADOOP-3349. A file rename was incorrectly changing the name inside a

lease record. (Tsz Wo (Nicholas), SZE via dhruba)



git-svn-id: https://svn.apache.org/repos/asf/hadoop/core/trunk@655410 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -219,6 +219,9 @@ Trunk (unreleased changes)
     HADOOP-3371. Ignore InstanceAlreadyExistsException from
     MBeanUtil::registerMBean. (lohit vijayarenu via cdouglas)
 
+    HADOOP-3349. A file rename was incorrectly changing the name inside a
+    lease record. (Tsz Wo (Nicholas), SZE via dhruba)
+
 Release 0.17.0 - Unreleased
 
   INCOMPATIBLE CHANGES

src/java/org/apache/hadoop/dfs/FSNamesystem.java

@@ -895,7 +895,8 @@ class FSNamesystem implements FSConstants, FSNamesystemMBean {
         // to proceed. Otherwise, prevent this request from creating file.
         //
         if (lease.expiredSoftLimit()) {
-          leaseManager.handleExpiredSoftLimit(lease);
+          LOG.info("startFile: Removing lease " + lease);
+          leaseManager.removeExpiredLease(lease);
         } else {
           throw new AlreadyBeingCreatedException(
                                                  "failed to create file " + src + " for " + holder +

src/java/org/apache/hadoop/dfs/LeaseManager.java

@@ -22,6 +22,8 @@ import java.util.*;
 
 import org.apache.commons.logging.Log;
 
+import org.apache.hadoop.fs.Path;
+
 class LeaseManager {
   static final Log LOG = FSNamesystem.LOG;
 
@@ -109,10 +111,9 @@ class LeaseManager {
     }
   }
 
-  synchronized void handleExpiredSoftLimit(Lease lease) throws IOException {
+  synchronized void removeExpiredLease(Lease lease) throws IOException {
     lease.releaseLocks();
     leases.remove(lease.holder);
-    LOG.info("startFile: Removing lease " + lease);
     if (!sortedLeases.remove(lease)) {
       LOG.error("startFile: Unknown failure trying to remove " + lease + 
                 " from lease set.");
@@ -268,10 +269,11 @@ class LeaseManager {
 
     Map<String, Lease> addTo = new TreeMap<String, Lease>();
     SortedMap<String, Lease> myset = sortedLeasesByPath.tailMap(src);
+    int srclen = src.length();
     for (Iterator<Map.Entry<String, Lease>> iter = myset.entrySet().iterator(); 
          iter.hasNext();) {
-      Map.Entry<String, Lease> value = iter.next();
-      String path = (String)value.getKey();
+      Map.Entry<String, Lease> entry = iter.next();
+      String path = entry.getKey();
       if (!path.startsWith(src)) {
         if (LOG.isDebugEnabled()) {
           LOG.debug("changelease comparing " + path +
@@ -279,8 +281,11 @@ class LeaseManager {
         }
         break;
       }
-      Lease lease = (Lease)value.getValue();
+      if (path.length() > srclen && path.charAt(srclen) != Path.SEPARATOR_CHAR){
+        continue;
+      }
 
+      Lease lease = entry.getValue();
       // Fix up all the pathnames in this lease.
       if (LOG.isDebugEnabled()) {
         LOG.debug("changelease comparing " + path +
@@ -314,19 +319,14 @@ class LeaseManager {
       try {
         while (fsnamesystem.fsRunning) {
           synchronized (fsnamesystem) {
-            synchronized (sortedLeases) {
+            synchronized (LeaseManager.this) {
               Lease top;
               while ((sortedLeases.size() > 0) &&
                      ((top = sortedLeases.first()) != null)) {
                 if (top.expiredHardLimit()) {
-                  top.releaseLocks();
-                  leases.remove(top.holder);
-                  LOG.info("Removing lease " + top
+                  LOG.info("Lease Monitor: Removing lease " + top
                       + ", leases remaining: " + sortedLeases.size());
-                  if (!sortedLeases.remove(top)) {
-                    LOG.warn("Unknown failure trying to remove " + top
-                        + " from lease set.");
-                  }
+                  removeExpiredLease(top);
                 } else {
                   break;
                 }

src/test/org/apache/hadoop/dfs/TestDFSRename.java

@@ -0,0 +1,79 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.dfs;
+
+import java.io.*;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+
+public class TestDFSRename extends junit.framework.TestCase {
+  static int countLease(MiniDFSCluster cluster) {
+    return cluster.getNameNode().namesystem.leaseManager.countLease();
+  }
+  
+  final Path dir = new Path("/test/rename/");
+
+  void list(FileSystem fs, String name) throws IOException {
+    FileSystem.LOG.info("\n\n" + name);
+    for(FileStatus s : fs.listStatus(dir)) {
+      FileSystem.LOG.info("" + s.getPath());
+    }
+  }
+
+  public void testRename() throws Exception {
+    Configuration conf = new Configuration();
+    MiniDFSCluster cluster = new MiniDFSCluster(conf, 2, true, null);
+    try {
+      FileSystem fs = cluster.getFileSystem();
+      assertTrue(fs.mkdirs(dir));
+      
+      Path a = new Path(dir, "a");
+      Path aa = new Path(dir, "aa");
+      Path b = new Path(dir, "b");
+
+      DataOutputStream a_out = fs.create(a);
+      a_out.writeBytes("something");
+      a_out.close();
+      
+      //should not have any lease
+      assertEquals(0, countLease(cluster)); 
+
+      DataOutputStream aa_out = fs.create(aa);
+      aa_out.writeBytes("something");
+
+      //should have 1 lease
+      assertEquals(1, countLease(cluster)); 
+      list(fs, "rename0");
+      fs.rename(a, b);
+      list(fs, "rename1");
+      aa_out.writeBytes(" more");
+      aa_out.close();
+      list(fs, "rename2");
+
+      //should not have any lease
+      assertEquals(0, countLease(cluster)); 
+
+      fs.delete(dir, true);
+    } finally {
+      if (cluster != null) {cluster.shutdown();}
+    }
+  }
+}