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 ay...@apache.org on 2019/04/05 03:01:58 UTC
[hadoop] branch HDFS-13891 updated: HDFS-13853. RBF: RouterAdmin
update cmd is overwriting the entry not updating the existing. Contributed
by Ayush Saxena.
This is an automated email from the ASF dual-hosted git repository.
ayushsaxena pushed a commit to branch HDFS-13891
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/HDFS-13891 by this push:
new 007b8ea HDFS-13853. RBF: RouterAdmin update cmd is overwriting the entry not updating the existing. Contributed by Ayush Saxena.
007b8ea is described below
commit 007b8ea1a5a020162582968b527cf78567dc4e97
Author: Ayush Saxena <ay...@apache.org>
AuthorDate: Fri Apr 5 08:11:16 2019 +0530
HDFS-13853. RBF: RouterAdmin update cmd is overwriting the entry not updating the existing. Contributed by Ayush Saxena.
---
.../hadoop/hdfs/tools/federation/RouterAdmin.java | 219 +++++++++++----------
.../federation/router/TestRouterAdminCLI.java | 130 ++++++++++--
.../hadoop-hdfs/src/site/markdown/HDFSCommands.md | 4 +-
3 files changed, 232 insertions(+), 121 deletions(-)
diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
index 61da7e9..9d03a44 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
+++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java
@@ -22,8 +22,10 @@ import java.net.InetSocketAddress;
import java.util.Arrays;
import java.util.Collection;
import java.util.LinkedHashMap;
+import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import org.apache.hadoop.classification.InterfaceAudience.Private;
import org.apache.hadoop.conf.Configuration;
@@ -138,9 +140,10 @@ public class RouterAdmin extends Configured implements Tool {
+ "[-readonly] [-faulttolerant] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
+ "-owner <owner> -group <group> -mode <mode>]";
} else if (cmd.equals("-update")) {
- return "\t[-update <source> <nameservice1, nameservice2, ...> "
- + "<destination> "
- + "[-readonly] [-faulttolerant] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
+ return "\t[-update <source>"
+ + " [<nameservice1, nameservice2, ...> <destination>] "
+ + "[-readonly true|false] [-faulttolerant true|false]"
+ + " [-order HASH|LOCAL|RANDOM|HASH_ALL] "
+ "-owner <owner> -group <group> -mode <mode>]";
} else if (cmd.equals("-rm")) {
return "\t[-rm <source>]";
@@ -294,6 +297,8 @@ public class RouterAdmin extends Configured implements Tool {
} else if ("-update".equals(cmd)) {
if (updateMount(argv, i)) {
System.out.println("Successfully updated mount point " + argv[i]);
+ System.out.println(
+ "WARN: Changing order/destinations may lead to inconsistencies");
} else {
exitCode = -1;
}
@@ -366,6 +371,10 @@ public class RouterAdmin extends Configured implements Tool {
e.printStackTrace();
debugException = ex;
}
+ } catch (IOException ioe) {
+ exitCode = -1;
+ System.err.println(cmd.substring(1) + ": " + ioe.getLocalizedMessage());
+ printUsage(cmd);
} catch (Exception e) {
exitCode = -1;
debugException = e;
@@ -473,17 +482,7 @@ public class RouterAdmin extends Configured implements Tool {
mount = normalizeFileSystemPath(mount);
// Get the existing entry
MountTableManager mountTable = client.getMountTableManager();
- GetMountTableEntriesRequest getRequest =
- GetMountTableEntriesRequest.newInstance(mount);
- GetMountTableEntriesResponse getResponse =
- mountTable.getMountTableEntries(getRequest);
- List<MountTable> results = getResponse.getEntries();
- MountTable existingEntry = null;
- for (MountTable result : results) {
- if (mount.equals(result.getSourcePath())) {
- existingEntry = result;
- }
- }
+ MountTable existingEntry = getMountEntry(mount, mountTable);
if (existingEntry == null) {
// Create and add the entry if it doesn't exist
@@ -579,100 +578,81 @@ public class RouterAdmin extends Configured implements Tool {
* @throws IOException If there is an error.
*/
public boolean updateMount(String[] parameters, int i) throws IOException {
- // Mandatory parameters
String mount = parameters[i++];
- String[] nss = parameters[i++].split(",");
- String dest = parameters[i++];
-
- // Optional parameters
- boolean readOnly = false;
- boolean faultTolerant = false;
- String owner = null;
- String group = null;
- FsPermission mode = null;
- DestinationOrder order = null;
- while (i < parameters.length) {
- if (parameters[i].equals("-readonly")) {
- readOnly = true;
- } else if (parameters[i].equals("-faulttolerant")) {
- faultTolerant = true;
- } else if (parameters[i].equals("-order")) {
- i++;
- try {
- order = DestinationOrder.valueOf(parameters[i]);
- } catch(Exception e) {
- System.err.println("Cannot parse order: " + parameters[i]);
- }
- } else if (parameters[i].equals("-owner")) {
- i++;
- owner = parameters[i];
- } else if (parameters[i].equals("-group")) {
- i++;
- group = parameters[i];
- } else if (parameters[i].equals("-mode")) {
- i++;
- short modeValue = Short.parseShort(parameters[i], 8);
- mode = new FsPermission(modeValue);
- } else {
- printUsage("-update");
- return false;
- }
-
- i++;
- }
-
- return updateMount(mount, nss, dest, readOnly, faultTolerant, order,
- new ACLEntity(owner, group, mode));
- }
-
- /**
- * Update a mount table entry.
- *
- * @param mount Mount point.
- * @param nss Nameservices where this is mounted to.
- * @param dest Destination path.
- * @param readonly If the mount point is read only.
- * @param order Order of the destination locations.
- * @param aclInfo the ACL info for mount point.
- * @return If the mount point was updated.
- * @throws IOException Error updating the mount point.
- */
- public boolean updateMount(String mount, String[] nss, String dest,
- boolean readonly, boolean faultTolerant,
- DestinationOrder order, ACLEntity aclInfo)
- throws IOException {
mount = normalizeFileSystemPath(mount);
MountTableManager mountTable = client.getMountTableManager();
-
- // Create a new entry
- Map<String, String> destMap = new LinkedHashMap<>();
- for (String ns : nss) {
- destMap.put(ns, dest);
- }
- MountTable newEntry = MountTable.newInstance(mount, destMap);
-
- newEntry.setReadOnly(readonly);
- newEntry.setFaultTolerant(faultTolerant);
-
- if (order != null) {
- newEntry.setDestOrder(order);
- }
-
- // Update ACL info of mount table entry
- if (aclInfo.getOwner() != null) {
- newEntry.setOwnerName(aclInfo.getOwner());
+ MountTable existingEntry = getMountEntry(mount, mountTable);
+ if (existingEntry == null) {
+ throw new IOException(mount + " doesn't exist.");
}
+ // Check if the destination needs to be updated.
- if (aclInfo.getGroup() != null) {
- newEntry.setGroupName(aclInfo.getGroup());
+ if (!parameters[i].startsWith("-")) {
+ String[] nss = parameters[i++].split(",");
+ String dest = parameters[i++];
+ Map<String, String> destMap = new LinkedHashMap<>();
+ for (String ns : nss) {
+ destMap.put(ns, dest);
+ }
+ final List<RemoteLocation> locations = new LinkedList<>();
+ for (Entry<String, String> entry : destMap.entrySet()) {
+ String nsId = entry.getKey();
+ String path = normalizeFileSystemPath(entry.getValue());
+ RemoteLocation location = new RemoteLocation(nsId, path, mount);
+ locations.add(location);
+ }
+ existingEntry.setDestinations(locations);
}
-
- if (aclInfo.getMode() != null) {
- newEntry.setMode(aclInfo.getMode());
+ try {
+ while (i < parameters.length) {
+ switch (parameters[i]) {
+ case "-readonly":
+ i++;
+ existingEntry.setReadOnly(getBooleanValue(parameters[i]));
+ break;
+ case "-faulttolerant":
+ i++;
+ existingEntry.setFaultTolerant(getBooleanValue(parameters[i]));
+ break;
+ case "-order":
+ i++;
+ try {
+ existingEntry.setDestOrder(DestinationOrder.valueOf(parameters[i]));
+ break;
+ } catch (Exception e) {
+ throw new Exception("Cannot parse order: " + parameters[i]);
+ }
+ case "-owner":
+ i++;
+ existingEntry.setOwnerName(parameters[i]);
+ break;
+ case "-group":
+ i++;
+ existingEntry.setGroupName(parameters[i]);
+ break;
+ case "-mode":
+ i++;
+ short modeValue = Short.parseShort(parameters[i], 8);
+ existingEntry.setMode(new FsPermission(modeValue));
+ break;
+ default:
+ printUsage("-update");
+ return false;
+ }
+ i++;
+ }
+ } catch (IllegalArgumentException iae) {
+ throw iae;
+ } catch (Exception e) {
+ String msg = "Unable to parse arguments: " + e.getMessage();
+ if (e instanceof ArrayIndexOutOfBoundsException) {
+ msg = "Unable to parse arguments: no value provided for "
+ + parameters[i - 1];
+ }
+ throw new IOException(msg);
}
-
UpdateMountTableEntryRequest updateRequest =
- UpdateMountTableEntryRequest.newInstance(newEntry);
+ UpdateMountTableEntryRequest.newInstance(existingEntry);
UpdateMountTableEntryResponse updateResponse =
mountTable.updateMountTableEntry(updateRequest);
boolean updated = updateResponse.getStatus();
@@ -683,6 +663,45 @@ public class RouterAdmin extends Configured implements Tool {
}
/**
+ * Parse string to boolean.
+ * @param value the string to be parsed.
+ * @return parsed boolean value.
+ * @throws Exception if other than true|false is provided.
+ */
+ private boolean getBooleanValue(String value) throws Exception {
+ if (value.equalsIgnoreCase("true")) {
+ return true;
+ } else if (value.equalsIgnoreCase("false")) {
+ return false;
+ }
+ throw new IllegalArgumentException("Invalid argument: " + value
+ + ". Please specify either true or false.");
+ }
+
+ /**
+ * Gets the mount table entry.
+ * @param mount name of the mount entry.
+ * @param mountTable the mount table.
+ * @return corresponding mount entry.
+ * @throws IOException in case of failure to retrieve mount entry.
+ */
+ private MountTable getMountEntry(String mount, MountTableManager mountTable)
+ throws IOException {
+ GetMountTableEntriesRequest getRequest =
+ GetMountTableEntriesRequest.newInstance(mount);
+ GetMountTableEntriesResponse getResponse =
+ mountTable.getMountTableEntries(getRequest);
+ List<MountTable> results = getResponse.getEntries();
+ MountTable existingEntry = null;
+ for (MountTable result : results) {
+ if (mount.equals(result.getSourcePath())) {
+ existingEntry = result;
+ }
+ }
+ return existingEntry;
+ }
+
+ /**
* Remove mount point.
*
* @param path Path to remove.
diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java
index 381203b..ce260ec 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java
+++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java
@@ -536,8 +536,8 @@ public class TestRouterAdminCLI {
argv = new String[] {"-update", src, nsId};
assertEquals(-1, ToolRunner.run(admin, argv));
assertTrue("Wrong message: " + out, out.toString().contains(
- "\t[-update <source> <nameservice1, nameservice2, ...> <destination> "
- + "[-readonly] [-faulttolerant] "
+ "\t[-update <source> [<nameservice1, nameservice2, ...> <destination>] "
+ + "[-readonly true|false] [-faulttolerant true|false] "
+ "[-order HASH|LOCAL|RANDOM|HASH_ALL] "
+ "-owner <owner> -group <group> -mode <mode>]"));
out.reset();
@@ -581,9 +581,9 @@ public class TestRouterAdminCLI {
+ "\t[-add <source> <nameservice1, nameservice2, ...> <destination> "
+ "[-readonly] [-faulttolerant] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
+ "-owner <owner> -group <group> -mode <mode>]\n"
- + "\t[-update <source> <nameservice1, nameservice2, ...> "
- + "<destination> "
- + "[-readonly] [-faulttolerant] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
+ + "\t[-update <source> [<nameservice1, nameservice2, ...> "
+ + "<destination>] [-readonly true|false]"
+ + " [-faulttolerant true|false] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
+ "-owner <owner> -group <group> -mode <mode>]\n" + "\t[-rm <source>]\n"
+ "\t[-ls <path>]\n"
+ "\t[-getDestination <path>]\n"
@@ -902,23 +902,15 @@ public class TestRouterAdminCLI {
@Test
public void testUpdateNonExistingMountTable() throws Exception {
- System.setOut(new PrintStream(out));
+ System.setErr(new PrintStream(err));
String nsId = "ns0";
String src = "/test-updateNonExistingMounttable";
String dest = "/updateNonExistingMounttable";
String[] argv = new String[] {"-update", src, nsId, dest};
- assertEquals(0, ToolRunner.run(admin, argv));
-
- stateStore.loadCache(MountTableStoreImpl.class, true);
- GetMountTableEntriesRequest getRequest =
- GetMountTableEntriesRequest.newInstance(src);
- GetMountTableEntriesResponse getResponse =
- client.getMountTableManager().getMountTableEntries(getRequest);
- // Ensure the destination updated successfully
- MountTable mountTable = getResponse.getEntries().get(0);
- assertEquals(src, mountTable.getSourcePath());
- assertEquals(nsId, mountTable.getDestinations().get(0).getNameserviceId());
- assertEquals(dest, mountTable.getDestinations().get(0).getDest());
+ // Update shall fail if the mount entry doesn't exist.
+ assertEquals(-1, ToolRunner.run(admin, argv));
+ assertTrue(err.toString(), err.toString()
+ .contains("update: /test-updateNonExistingMounttable doesn't exist."));
}
@Test
@@ -998,6 +990,106 @@ public class TestRouterAdminCLI {
}
@Test
+ public void testUpdateChangeAttributes() throws Exception {
+ // Add a mount table firstly
+ String nsId = "ns0";
+ String src = "/mount";
+ String dest = "/dest";
+ String[] argv = new String[] {"-add", src, nsId, dest, "-readonly",
+ "-order", "HASH_ALL"};
+ assertEquals(0, ToolRunner.run(admin, argv));
+
+ stateStore.loadCache(MountTableStoreImpl.class, true);
+ GetMountTableEntriesRequest getRequest =
+ GetMountTableEntriesRequest.newInstance(src);
+ GetMountTableEntriesResponse getResponse =
+ client.getMountTableManager().getMountTableEntries(getRequest);
+ // Ensure mount table added successfully
+ MountTable mountTable = getResponse.getEntries().get(0);
+ assertEquals(src, mountTable.getSourcePath());
+
+ // Update the destination
+ String newNsId = "ns0";
+ String newDest = "/newDestination";
+ argv = new String[] {"-update", src, newNsId, newDest};
+ assertEquals(0, ToolRunner.run(admin, argv));
+
+ stateStore.loadCache(MountTableStoreImpl.class, true);
+ getResponse =
+ client.getMountTableManager().getMountTableEntries(getRequest);
+ // Ensure the destination updated successfully and other attributes are
+ // preserved.
+ mountTable = getResponse.getEntries().get(0);
+ assertEquals(src, mountTable.getSourcePath());
+ assertEquals(newNsId,
+ mountTable.getDestinations().get(0).getNameserviceId());
+ assertEquals(newDest, mountTable.getDestinations().get(0).getDest());
+ assertTrue(mountTable.isReadOnly());
+ assertEquals("HASH_ALL", mountTable.getDestOrder().toString());
+
+ // Update the attribute.
+ argv = new String[] {"-update", src, "-readonly", "false"};
+ assertEquals(0, ToolRunner.run(admin, argv));
+
+ stateStore.loadCache(MountTableStoreImpl.class, true);
+ getResponse =
+ client.getMountTableManager().getMountTableEntries(getRequest);
+
+ // Ensure the attribute updated successfully and destination and other
+ // attributes are preserved.
+ mountTable = getResponse.getEntries().get(0);
+ assertEquals(src, mountTable.getSourcePath());
+ assertEquals(newNsId,
+ mountTable.getDestinations().get(0).getNameserviceId());
+ assertEquals(newDest, mountTable.getDestinations().get(0).getDest());
+ assertFalse(mountTable.isReadOnly());
+ assertEquals("HASH_ALL", mountTable.getDestOrder().toString());
+
+ }
+
+ @Test
+ public void testUpdateErrorCase() throws Exception {
+ // Add a mount table firstly
+ String nsId = "ns0";
+ String src = "/mount";
+ String dest = "/dest";
+ String[] argv = new String[] {"-add", src, nsId, dest, "-readonly",
+ "-order", "HASH_ALL"};
+ assertEquals(0, ToolRunner.run(admin, argv));
+ stateStore.loadCache(MountTableStoreImpl.class, true);
+
+ // Check update for non-existent mount entry.
+ argv = new String[] {"-update", "/noMount", "-readonly", "false"};
+ System.setErr(new PrintStream(err));
+ assertEquals(-1, ToolRunner.run(admin, argv));
+ assertTrue(err.toString(),
+ err.toString().contains("update: /noMount doesn't exist."));
+ err.reset();
+
+ // Check update if non true/false value is passed for readonly.
+ argv = new String[] {"-update", src, "-readonly", "check"};
+ assertEquals(-1, ToolRunner.run(admin, argv));
+ assertTrue(err.toString(), err.toString().contains("update: "
+ + "Invalid argument: check. Please specify either true or false."));
+ err.reset();
+
+ // Check update with missing value is passed for faulttolerant.
+ argv = new String[] {"-update", src, "ns1", "/tmp", "-faulttolerant"};
+ assertEquals(-1, ToolRunner.run(admin, argv));
+ assertTrue(err.toString(),
+ err.toString().contains("update: Unable to parse arguments:"
+ + " no value provided for -faulttolerant"));
+ err.reset();
+
+ // Check update with invalid order.
+ argv = new String[] {"-update", src, "ns1", "/tmp", "-order", "Invalid"};
+ assertEquals(-1, ToolRunner.run(admin, argv));
+ assertTrue(err.toString(), err.toString().contains(
+ "update: Unable to parse arguments: Cannot parse order: Invalid"));
+ err.reset();
+ }
+
+ @Test
public void testUpdateReadonlyUserGroupPermissionMountable()
throws Exception {
// Add a mount table
@@ -1022,7 +1114,7 @@ public class TestRouterAdminCLI {
// Update the readonly, owner, group and permission
String testOwner = "test_owner";
String testGroup = "test_group";
- argv = new String[] {"-update", src, nsId, dest, "-readonly",
+ argv = new String[] {"-update", src, nsId, dest, "-readonly", "true",
"-owner", testOwner, "-group", testGroup, "-mode", "0455"};
assertEquals(0, ToolRunner.run(admin, argv));
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSCommands.md b/hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSCommands.md
index 6336281..dc4c10a 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSCommands.md
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSCommands.md
@@ -429,7 +429,7 @@ Usage:
hdfs dfsrouteradmin
[-add <source> <nameservice1, nameservice2, ...> <destination> [-readonly] [-faulttolerant] [-order HASH|LOCAL|RANDOM|HASH_ALL] -owner <owner> -group <group> -mode <mode>]
- [-update <source> <nameservice1, nameservice2, ...> <destination> [-readonly] [-faulttolerant] [-order HASH|LOCAL|RANDOM|HASH_ALL] -owner <owner> -group <group> -mode <mode>]
+ [-update <source> [<nameservice1, nameservice2, ...> <destination>] [-readonly true|false] [-faulttolerant true|false] [-order HASH|LOCAL|RANDOM|HASH_ALL] -owner <owner> -group <group> -mode <mode>]
[-rm <source>]
[-ls <path>]
[-getDestination <path>]
@@ -444,7 +444,7 @@ Usage:
| COMMAND\_OPTION | Description |
|:---- |:---- |
| `-add` *source* *nameservices* *destination* | Add a mount table entry or update if it exists. |
-| `-update` *source* *nameservices* *destination* | Update a mount table entry or create one if it does not exist. |
+| `-update` *source* *nameservices* *destination* | Update a mount table entry attribures. |
| `-rm` *source* | Remove mount point of specified path. |
| `-ls` *path* | List mount points under specified path. |
| `-getDestination` *path* | Get the subcluster where a file is or should be created. |
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org