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 sz...@apache.org on 2015/06/16 01:27:38 UTC
hadoop git commit: HDFS-8540. Mover should exit with NO_MOVE_BLOCK if
no block can be moved. Contributed by surendra singh lilhore
Repository: hadoop
Updated Branches:
refs/heads/trunk 2cb09e98e -> 321940cf1
HDFS-8540. Mover should exit with NO_MOVE_BLOCK if no block can be moved. Contributed by surendra singh lilhore
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/321940cf
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/321940cf
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/321940cf
Branch: refs/heads/trunk
Commit: 321940cf19375febe9660e96d905360cfcc15f5f
Parents: 2cb09e9
Author: Tsz-Wo Nicholas Sze <sz...@hortonworks.com>
Authored: Mon Jun 15 16:26:53 2015 -0700
Committer: Tsz-Wo Nicholas Sze <sz...@hortonworks.com>
Committed: Mon Jun 15 16:26:53 2015 -0700
----------------------------------------------------------------------
hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 +
.../apache/hadoop/hdfs/server/mover/Mover.java | 95 ++++++++++++++------
.../hadoop/hdfs/server/mover/TestMover.java | 29 ++++++
.../hdfs/server/mover/TestStorageMover.java | 18 ++--
4 files changed, 107 insertions(+), 38 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/321940cf/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 21acf98..584d94d 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
+++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
@@ -917,6 +917,9 @@ Release 2.7.1 - UNRELEASED
HDFS-8521. Add VisibleForTesting annotation to
BlockPoolSlice#selectReplicaToDelete. (cmccabe)
+ HDFS-8540. Mover should exit with NO_MOVE_BLOCK if no block can be moved.
+ (surendra singh lilhore via szetszwo)
+
OPTIMIZATIONS
BUG FIXES
http://git-wip-us.apache.org/repos/asf/hadoop/blob/321940cf/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/mover/Mover.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/mover/Mover.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/mover/Mover.java
index 8715ce4..344b9fc 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/mover/Mover.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/mover/Mover.java
@@ -27,7 +27,6 @@ import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.conf.Configured;
-import org.apache.hadoop.fs.BlockStoragePolicySpi;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.StorageType;
import org.apache.hadoop.hdfs.HdfsConfiguration;
@@ -163,8 +162,7 @@ public class Mover {
private ExitStatus run() {
try {
init();
- boolean hasRemaining = new Processor().processNamespace();
- return hasRemaining ? ExitStatus.IN_PROGRESS : ExitStatus.SUCCESS;
+ return new Processor().processNamespace().getExitStatus();
} catch (IllegalArgumentException e) {
System.out.println(e + ". Exiting ...");
return ExitStatus.ILLEGAL_ARGUMENTS;
@@ -262,11 +260,11 @@ public class Mover {
* @return whether there is still remaining migration work for the next
* round
*/
- private boolean processNamespace() throws IOException {
+ private Result processNamespace() throws IOException {
getSnapshottableDirs();
- boolean hasRemaining = false;
+ Result result = new Result();
for (Path target : targetPaths) {
- hasRemaining |= processPath(target.toUri().getPath());
+ processPath(target.toUri().getPath(), result);
}
// wait for pending move to finish and retry the failed migration
boolean hasFailed = Dispatcher.waitForMoveCompletion(storages.targets
@@ -282,16 +280,15 @@ public class Mover {
// Reset retry count if no failure.
retryCount.set(0);
}
- hasRemaining |= hasFailed;
- return hasRemaining;
+ result.updateHasRemaining(hasFailed);
+ return result;
}
/**
* @return whether there is still remaing migration work for the next
* round
*/
- private boolean processPath(String fullPath) {
- boolean hasRemaining = false;
+ private void processPath(String fullPath, Result result) {
for (byte[] lastReturnedName = HdfsFileStatus.EMPTY_NAME;;) {
final DirectoryListing children;
try {
@@ -299,73 +296,71 @@ public class Mover {
} catch(IOException e) {
LOG.warn("Failed to list directory " + fullPath
+ ". Ignore the directory and continue.", e);
- return hasRemaining;
+ return;
}
if (children == null) {
- return hasRemaining;
+ return;
}
for (HdfsFileStatus child : children.getPartialListing()) {
- hasRemaining |= processRecursively(fullPath, child);
+ processRecursively(fullPath, child, result);
}
if (children.hasMore()) {
lastReturnedName = children.getLastName();
} else {
- return hasRemaining;
+ return;
}
}
}
/** @return whether the migration requires next round */
- private boolean processRecursively(String parent, HdfsFileStatus status) {
+ private void processRecursively(String parent, HdfsFileStatus status,
+ Result result) {
String fullPath = status.getFullName(parent);
- boolean hasRemaining = false;
if (status.isDir()) {
if (!fullPath.endsWith(Path.SEPARATOR)) {
fullPath = fullPath + Path.SEPARATOR;
}
- hasRemaining = processPath(fullPath);
+ processPath(fullPath, result);
// process snapshots if this is a snapshottable directory
if (snapshottableDirs.contains(fullPath)) {
final String dirSnapshot = fullPath + HdfsConstants.DOT_SNAPSHOT_DIR;
- hasRemaining |= processPath(dirSnapshot);
+ processPath(dirSnapshot, result);
}
} else if (!status.isSymlink()) { // file
try {
if (!isSnapshotPathInCurrent(fullPath)) {
// the full path is a snapshot path but it is also included in the
// current directory tree, thus ignore it.
- hasRemaining = processFile(fullPath, (HdfsLocatedFileStatus)status);
+ processFile(fullPath, (HdfsLocatedFileStatus) status, result);
}
} catch (IOException e) {
LOG.warn("Failed to check the status of " + parent
+ ". Ignore it and continue.", e);
- return false;
}
}
- return hasRemaining;
}
/** @return true if it is necessary to run another round of migration */
- private boolean processFile(String fullPath, HdfsLocatedFileStatus status) {
+ private void processFile(String fullPath, HdfsLocatedFileStatus status,
+ Result result) {
final byte policyId = status.getStoragePolicy();
// currently we ignore files with unspecified storage policy
if (policyId == HdfsConstants.BLOCK_STORAGE_POLICY_ID_UNSPECIFIED) {
- return false;
+ return;
}
final BlockStoragePolicy policy = blockStoragePolicies[policyId];
if (policy == null) {
LOG.warn("Failed to get the storage policy of file " + fullPath);
- return false;
+ return;
}
final List<StorageType> types = policy.chooseStorageTypes(
status.getReplication());
final LocatedBlocks locatedBlocks = status.getBlockLocations();
- boolean hasRemaining = false;
final boolean lastBlkComplete = locatedBlocks.isLastBlockComplete();
List<LocatedBlock> lbs = locatedBlocks.getLocatedBlocks();
- for(int i = 0; i < lbs.size(); i++) {
+ for (int i = 0; i < lbs.size(); i++) {
if (i == lbs.size() - 1 && !lastBlkComplete) {
// last block is incomplete, skip it
continue;
@@ -375,12 +370,15 @@ public class Mover {
lb.getStorageTypes());
if (!diff.removeOverlap(true)) {
if (scheduleMoves4Block(diff, lb)) {
- hasRemaining |= (diff.existing.size() > 1 &&
- diff.expected.size() > 1);
+ result.updateHasRemaining(diff.existing.size() > 1
+ && diff.expected.size() > 1);
+ // One block scheduled successfully, set noBlockMoved to false
+ result.setNoBlockMoved(false);
+ } else {
+ result.updateHasRemaining(true);
}
}
}
- return hasRemaining;
}
boolean scheduleMoves4Block(StorageTypeDiff diff, LocatedBlock lb) {
@@ -711,6 +709,45 @@ public class Mover {
}
}
+ private static class Result {
+
+ private boolean hasRemaining;
+ private boolean noBlockMoved;
+
+ Result() {
+ hasRemaining = false;
+ noBlockMoved = true;
+ }
+
+ boolean isHasRemaining() {
+ return hasRemaining;
+ }
+
+ boolean isNoBlockMoved() {
+ return noBlockMoved;
+ }
+
+ void updateHasRemaining(boolean hasRemaining) {
+ this.hasRemaining |= hasRemaining;
+ }
+
+ void setNoBlockMoved(boolean noBlockMoved) {
+ this.noBlockMoved = noBlockMoved;
+ }
+
+ /**
+ * @return SUCCESS if all moves are success and there is no remaining move.
+ * Return NO_MOVE_BLOCK if there moves available but all the moves
+ * cannot be scheduled. Otherwise, return IN_PROGRESS since there
+ * must be some remaining moves.
+ */
+ ExitStatus getExitStatus() {
+ return !isHasRemaining() ? ExitStatus.SUCCESS
+ : isNoBlockMoved() ? ExitStatus.NO_MOVE_BLOCK
+ : ExitStatus.IN_PROGRESS;
+ }
+
+ }
/**
* Run a Mover in command line.
*
http://git-wip-us.apache.org/repos/asf/hadoop/blob/321940cf/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/mover/TestMover.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/mover/TestMover.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/mover/TestMover.java
index f4bedab..49e2b23 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/mover/TestMover.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/mover/TestMover.java
@@ -328,6 +328,35 @@ public class TestMover {
}
}
+ @Test(timeout = 300000)
+ public void testMoveWhenStoragePolicyNotSatisfying() throws Exception {
+ // HDFS-8147
+ final Configuration conf = new HdfsConfiguration();
+ final MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf)
+ .numDataNodes(3)
+ .storageTypes(
+ new StorageType[][] { { StorageType.DISK }, { StorageType.DISK },
+ { StorageType.DISK } }).build();
+ try {
+ cluster.waitActive();
+ final DistributedFileSystem dfs = cluster.getFileSystem();
+ final String file = "/testMoveWhenStoragePolicyNotSatisfying";
+ // write to DISK
+ final FSDataOutputStream out = dfs.create(new Path(file));
+ out.writeChars("testMoveWhenStoragePolicyNotSatisfying");
+ out.close();
+
+ // move to ARCHIVE
+ dfs.setStoragePolicy(new Path(file), "COLD");
+ int rc = ToolRunner.run(conf, new Mover.Cli(),
+ new String[] { "-p", file.toString() });
+ int exitcode = ExitStatus.NO_MOVE_BLOCK.getExitCode();
+ Assert.assertEquals("Exit code should be " + exitcode, exitcode, rc);
+ } finally {
+ cluster.shutdown();
+ }
+ }
+
@Test
public void testMoverFailedRetry() throws Exception {
// HDFS-8147
http://git-wip-us.apache.org/repos/asf/hadoop/blob/321940cf/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/mover/TestStorageMover.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/mover/TestStorageMover.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/mover/TestStorageMover.java
index d8b40d4..3095f30 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/mover/TestStorageMover.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/mover/TestStorageMover.java
@@ -219,7 +219,7 @@ public class TestStorageMover {
verify(true);
setStoragePolicy();
- migrate();
+ migrate(ExitStatus.SUCCESS);
verify(true);
} finally {
if (shutdown) {
@@ -250,8 +250,8 @@ public class TestStorageMover {
/**
* Run the migration tool.
*/
- void migrate() throws Exception {
- runMover();
+ void migrate(ExitStatus expectedExitCode) throws Exception {
+ runMover(expectedExitCode);
Thread.sleep(5000); // let the NN finish deletion
}
@@ -267,14 +267,14 @@ public class TestStorageMover {
}
}
- private void runMover() throws Exception {
+ private void runMover(ExitStatus expectedExitCode) throws Exception {
Collection<URI> namenodes = DFSUtil.getNsServiceRpcUris(conf);
Map<URI, List<Path>> nnMap = Maps.newHashMap();
for (URI nn : namenodes) {
nnMap.put(nn, null);
}
int result = Mover.run(nnMap, conf);
- Assert.assertEquals(ExitStatus.SUCCESS.getExitCode(), result);
+ Assert.assertEquals(expectedExitCode.getExitCode(), result);
}
private void verifyNamespace() throws Exception {
@@ -555,7 +555,7 @@ public class TestStorageMover {
try {
banner("start data migration");
test.setStoragePolicy(); // set /foo to COLD
- test.migrate();
+ test.migrate(ExitStatus.SUCCESS);
// make sure the under construction block has not been migrated
LocatedBlocks lbs = test.dfs.getClient().getLocatedBlocks(
@@ -605,7 +605,7 @@ public class TestStorageMover {
try {
test.runBasicTest(false);
pathPolicyMap.moveAround(test.dfs);
- test.migrate();
+ test.migrate(ExitStatus.SUCCESS);
test.verify(true);
} finally {
@@ -695,7 +695,7 @@ public class TestStorageMover {
//test move a hot file to warm
final Path file1 = new Path(pathPolicyMap.hot, "file1");
test.dfs.rename(file1, pathPolicyMap.warm);
- test.migrate();
+ test.migrate(ExitStatus.NO_MOVE_BLOCK);
test.verifyFile(new Path(pathPolicyMap.warm, "file1"), WARM.getId());
} finally {
test.shutdownCluster();
@@ -753,7 +753,7 @@ public class TestStorageMover {
{ //test move a cold file to warm
final Path file1 = new Path(pathPolicyMap.cold, "file1");
test.dfs.rename(file1, pathPolicyMap.warm);
- test.migrate();
+ test.migrate(ExitStatus.SUCCESS);
test.verify(true);
}
} finally {