HDFS-8576. Lease recovery should return true if the lease can be released and the file can be closed. Contributed by J.Andreina
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/9e1a876b Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/9e1a876b Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/9e1a876b Branch: refs/heads/YARN-2928 Commit: 9e1a876b1305f374379ab64d6cc3adffb30b4549 Parents: 0eb0d6e Author: Tsz-Wo Nicholas Sze <szets...@hortonworks.com> Authored: Mon Jun 15 16:07:38 2015 -0700 Committer: Zhijie Shen <zjs...@apache.org> Committed: Thu Jun 18 11:10:06 2015 -0700 ---------------------------------------------------------------------- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../hdfs/server/namenode/FSNamesystem.java | 22 ++++++---- .../apache/hadoop/hdfs/TestLeaseRecovery.java | 46 ++++++++++++++++++++ 3 files changed, 62 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/9e1a876b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index c98d918..21acf98 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -1011,6 +1011,9 @@ Release 2.7.1 - UNRELEASED HDFS-8595. TestCommitBlockSynchronization fails in branch-2.7. (Patch applies to all branches). (Arpit Agarwal) + HDFS-8576. Lease recovery should return true if the lease can be released + and the file can be closed. (J.Andreina via szetszwo) + Release 2.7.0 - 2015-04-20 INCOMPATIBLE CHANGES http://git-wip-us.apache.org/repos/asf/hadoop/blob/9e1a876b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index f962373..518adb4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -2616,7 +2616,8 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, * @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 + * @return true if the file is already closed or + * if the lease can be released and the file can be closed. * @throws IOException */ boolean recoverLease(String src, String holder, String clientMachine) @@ -2643,7 +2644,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, dir.checkPathAccess(pc, iip, FsAction.WRITE); } - recoverLeaseInternal(RecoverLeaseOp.RECOVER_LEASE, + return recoverLeaseInternal(RecoverLeaseOp.RECOVER_LEASE, iip, src, holder, clientMachine, true); } catch (StandbyException se) { skipSync = true; @@ -2656,7 +2657,6 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, getEditLog().logSync(); } } - return false; } enum RecoverLeaseOp { @@ -2672,12 +2672,12 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, } } - void recoverLeaseInternal(RecoverLeaseOp op, INodesInPath iip, + boolean recoverLeaseInternal(RecoverLeaseOp op, INodesInPath iip, String src, String holder, String clientMachine, boolean force) throws IOException { assert hasWriteLock(); INodeFile file = iip.getLastINode().asFile(); - if (file != null && file.isUnderConstruction()) { + if (file.isUnderConstruction()) { // // If the file is under construction , then it must be in our // leases. Find the appropriate lease record. @@ -2710,7 +2710,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, // close only the file src LOG.info("recoverLease: " + lease + ", src=" + src + " from client " + clientName); - internalReleaseLease(lease, src, iip, holder); + return internalReleaseLease(lease, src, iip, holder); } else { assert lease.getHolder().equals(clientName) : "Current lease holder " + lease.getHolder() + @@ -2722,11 +2722,13 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, if (lease.expiredSoftLimit()) { LOG.info("startFile: recover " + lease + ", src=" + src + " client " + clientName); - boolean isClosed = internalReleaseLease(lease, src, iip, null); - if(!isClosed) + if (internalReleaseLease(lease, src, iip, null)) { + return true; + } else { throw new RecoveryInProgressException( op.getExceptionMessage(src, holder, clientMachine, "lease recovery is in progress. Try again later.")); + } } else { final BlockInfo lastBlock = file.getLastBlock(); if (lastBlock != null @@ -2743,7 +2745,9 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, } } } - } + } else { + return true; + } } /** http://git-wip-us.apache.org/repos/asf/hadoop/blob/9e1a876b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery.java index 15580a5..c9f3842 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hdfs; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.File; import java.io.IOException; @@ -43,6 +44,7 @@ import org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.TestInterDatanodePr import org.apache.hadoop.hdfs.server.namenode.LeaseManager; import org.apache.hadoop.hdfs.server.namenode.NameNodeAdapter; import org.apache.hadoop.io.EnumSetWritable; +import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.security.UserGroupInformation; import org.junit.After; import org.junit.Test; @@ -212,4 +214,48 @@ public class TestLeaseRecovery { assertTrue("File should be closed", newdfs.recoverLease(file)); } + + /** + * Recover the lease on a file and append file from another client. + */ + @Test + public void testLeaseRecoveryAndAppend() throws Exception { + Configuration conf = new Configuration(); + try{ + cluster = new MiniDFSCluster.Builder(conf).numDataNodes(1).build(); + Path file = new Path("/testLeaseRecovery"); + DistributedFileSystem dfs = cluster.getFileSystem(); + + // create a file with 0 bytes + FSDataOutputStream out = dfs.create(file); + out.hflush(); + out.hsync(); + + // abort the original stream + ((DFSOutputStream) out.getWrappedStream()).abort(); + DistributedFileSystem newdfs = + (DistributedFileSystem) FileSystem.newInstance + (cluster.getConfiguration(0)); + + // Append to a file , whose lease is held by another client should fail + try { + newdfs.append(file); + fail("Append to a file(lease is held by another client) should fail"); + } catch (RemoteException e) { + assertTrue(e.getMessage().contains("file lease is currently owned")); + } + + // Lease recovery on first try should be successful + boolean recoverLease = newdfs.recoverLease(file); + assertTrue(recoverLease); + FSDataOutputStream append = newdfs.append(file); + append.write("test".getBytes()); + append.close(); + }finally{ + if (cluster != null) { + cluster.shutdown(); + cluster = null; + } + } + } }