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 yq...@apache.org on 2018/03/13 03:03:50 UTC
hadoop git commit: HDFS-13226. RBF: Throw the exception if mount
table entry validated failed. Contributed by maobaolong.
Repository: hadoop
Updated Branches:
refs/heads/trunk 7fab787de -> 19292bc26
HDFS-13226. RBF: Throw the exception if mount table entry validated failed. Contributed by maobaolong.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/19292bc2
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/19292bc2
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/19292bc2
Branch: refs/heads/trunk
Commit: 19292bc264cada5117ec76063d36cc88159afdf4
Parents: 7fab787
Author: Yiqun Lin <yq...@apache.org>
Authored: Tue Mar 13 11:03:31 2018 +0800
Committer: Yiqun Lin <yq...@apache.org>
Committed: Tue Mar 13 11:03:31 2018 +0800
----------------------------------------------------------------------
.../federation/store/records/BaseRecord.java | 16 ++++++--
.../store/records/MembershipState.java | 29 ++++++++-----
.../federation/store/records/MountTable.java | 42 +++++++++++--------
.../federation/store/records/RouterState.java | 9 ++--
.../federation/router/TestRouterAdminCLI.java | 38 +++++++++++++++--
.../store/records/TestMountTable.java | 43 ++++++++++++++++++++
6 files changed, 137 insertions(+), 40 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/19292bc2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/BaseRecord.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/BaseRecord.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/BaseRecord.java
index 79f99c8..d5e60ce 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/BaseRecord.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/BaseRecord.java
@@ -32,6 +32,10 @@ import org.apache.hadoop.util.Time;
* </ul>
*/
public abstract class BaseRecord implements Comparable<BaseRecord> {
+ public static final String ERROR_MSG_CREATION_TIME_NEGATIVE =
+ "The creation time for the record cannot be negative.";
+ public static final String ERROR_MSG_MODIFICATION_TIME_NEGATIVE =
+ "The modification time for the record cannot be negative.";
/**
* Set the modification time for the record.
@@ -193,11 +197,15 @@ public abstract class BaseRecord implements Comparable<BaseRecord> {
/**
* Validates the record. Called when the record is created, populated from the
- * state store, and before committing to the state store.
- * @return If the record is valid.
+ * state store, and before committing to the state store. If validate failed,
+ * there throws an exception.
*/
- public boolean validate() {
- return getDateCreated() > 0 && getDateModified() > 0;
+ public void validate() {
+ if (getDateCreated() <= 0) {
+ throw new IllegalArgumentException(ERROR_MSG_CREATION_TIME_NEGATIVE);
+ } else if (getDateModified() <= 0) {
+ throw new IllegalArgumentException(ERROR_MSG_MODIFICATION_TIME_NEGATIVE);
+ }
}
@Override
http://git-wip-us.apache.org/repos/asf/hadoop/blob/19292bc2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MembershipState.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MembershipState.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MembershipState.java
index ac0b22e..e33dedf 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MembershipState.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MembershipState.java
@@ -37,6 +37,14 @@ import org.apache.hadoop.hdfs.server.federation.store.driver.StateStoreSerialize
*/
public abstract class MembershipState extends BaseRecord
implements FederationNamenodeContext {
+ public static final String ERROR_MSG_NO_NS_SPECIFIED =
+ "Invalid registration, no nameservice specified ";
+ public static final String ERROR_MSG_NO_WEB_ADDR_SPECIFIED =
+ "Invalid registration, no web address specified ";
+ public static final String ERROR_MSG_NO_RPC_ADDR_SPECIFIED =
+ "Invalid registration, no rpc address specified ";
+ public static final String ERROR_MSG_NO_BP_SPECIFIED =
+ "Invalid registration, no block pool specified ";
/** Expiration time in ms for this entry. */
private static long expirationMs;
@@ -226,26 +234,25 @@ public abstract class MembershipState extends BaseRecord
* is missing required information.
*/
@Override
- public boolean validate() {
- boolean ret = super.validate();
+ public void validate() {
+ super.validate();
if (getNameserviceId() == null || getNameserviceId().length() == 0) {
- //LOG.error("Invalid registration, no nameservice specified " + this);
- ret = false;
+ throw new IllegalArgumentException(
+ ERROR_MSG_NO_NS_SPECIFIED + this);
}
if (getWebAddress() == null || getWebAddress().length() == 0) {
- //LOG.error("Invalid registration, no web address specified " + this);
- ret = false;
+ throw new IllegalArgumentException(
+ ERROR_MSG_NO_WEB_ADDR_SPECIFIED + this);
}
if (getRpcAddress() == null || getRpcAddress().length() == 0) {
- //LOG.error("Invalid registration, no rpc address specified " + this);
- ret = false;
+ throw new IllegalArgumentException(
+ ERROR_MSG_NO_RPC_ADDR_SPECIFIED + this);
}
if (!isBadState() &&
(getBlockPoolId().isEmpty() || getBlockPoolId().length() == 0)) {
- //LOG.error("Invalid registration, no block pool specified " + this);
- ret = false;
+ throw new IllegalArgumentException(
+ ERROR_MSG_NO_BP_SPECIFIED + this);
}
- return ret;
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/19292bc2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java
index 3a99f14..0eab043 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java
@@ -51,7 +51,18 @@ import org.slf4j.LoggerFactory;
public abstract class MountTable extends BaseRecord {
private static final Logger LOG = LoggerFactory.getLogger(MountTable.class);
-
+ public static final String ERROR_MSG_NO_SOURCE_PATH =
+ "Invalid entry, no source path specified ";
+ public static final String ERROR_MSG_MUST_START_WITH_BACK_SLASH =
+ "Invalid entry, all mount points must start with / ";
+ public static final String ERROR_MSG_NO_DEST_PATH_SPECIFIED =
+ "Invalid entry, no destination paths specified ";
+ public static final String ERROR_MSG_INVAILD_DEST_NS =
+ "Invalid entry, invalid destination nameservice ";
+ public static final String ERROR_MSG_INVAILD_DEST_PATH =
+ "Invalid entry, invalid destination path ";
+ public static final String ERROR_MSG_ALL_DEST_MUST_START_WITH_BACK_SLASH =
+ "Invalid entry, all destination must start with / ";
/** Comparator for paths which considers the /. */
public static final Comparator<String> PATH_COMPARATOR =
@@ -342,36 +353,35 @@ public abstract class MountTable extends BaseRecord {
}
@Override
- public boolean validate() {
- boolean ret = super.validate();
+ public void validate() {
+ super.validate();
if (this.getSourcePath() == null || this.getSourcePath().length() == 0) {
- LOG.error("Invalid entry, no source path specified ", this);
- ret = false;
+ throw new IllegalArgumentException(
+ ERROR_MSG_NO_SOURCE_PATH + this);
}
if (!this.getSourcePath().startsWith("/")) {
- LOG.error("Invalid entry, all mount points must start with / ", this);
- ret = false;
+ throw new IllegalArgumentException(
+ ERROR_MSG_MUST_START_WITH_BACK_SLASH + this);
}
if (this.getDestinations() == null || this.getDestinations().size() == 0) {
- LOG.error("Invalid entry, no destination paths specified ", this);
- ret = false;
+ throw new IllegalArgumentException(
+ ERROR_MSG_NO_DEST_PATH_SPECIFIED + this);
}
for (RemoteLocation loc : getDestinations()) {
String nsId = loc.getNameserviceId();
if (nsId == null || nsId.length() == 0) {
- LOG.error("Invalid entry, invalid destination nameservice ", this);
- ret = false;
+ throw new IllegalArgumentException(
+ ERROR_MSG_INVAILD_DEST_NS + this);
}
if (loc.getDest() == null || loc.getDest().length() == 0) {
- LOG.error("Invalid entry, invalid destination path ", this);
- ret = false;
+ throw new IllegalArgumentException(
+ ERROR_MSG_INVAILD_DEST_PATH + this);
}
if (!loc.getDest().startsWith("/")) {
- LOG.error("Invalid entry, all destination must start with / ", this);
- ret = false;
+ throw new IllegalArgumentException(
+ ERROR_MSG_ALL_DEST_MUST_START_WITH_BACK_SLASH + this);
}
}
- return ret;
}
@Override
http://git-wip-us.apache.org/repos/asf/hadoop/blob/19292bc2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/RouterState.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/RouterState.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/RouterState.java
index d727395..c90abcc 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/RouterState.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/RouterState.java
@@ -127,14 +127,13 @@ public abstract class RouterState extends BaseRecord {
}
@Override
- public boolean validate() {
- boolean ret = super.validate();
+ public void validate() {
+ super.validate();
if ((getAddress() == null || getAddress().length() == 0) &&
getStatus() != RouterServiceState.INITIALIZING) {
- LOG.error("Invalid router entry, no address specified {}", this);
- ret = false;
+ throw new IllegalArgumentException(
+ "Invalid router entry, no address specified " + this);
}
- return ret;
}
@Override
http://git-wip-us.apache.org/repos/asf/hadoop/blob/19292bc2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java
index ce6e43a..161e613 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java
@@ -44,7 +44,6 @@ import org.apache.hadoop.test.GenericTestUtils;
import org.apache.hadoop.util.ToolRunner;
import org.junit.After;
import org.junit.AfterClass;
-import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -110,7 +109,7 @@ public class TestRouterAdminCLI {
String src = "/test-addmounttable";
String dest = "/addmounttable";
String[] argv = new String[] {"-add", src, nsId, dest};
- Assert.assertEquals(0, ToolRunner.run(admin, argv));
+ assertEquals(0, ToolRunner.run(admin, argv));
stateStore.loadCache(MountTableStoreImpl.class, true);
GetMountTableEntriesRequest getRequest = GetMountTableEntriesRequest
@@ -130,7 +129,7 @@ public class TestRouterAdminCLI {
// test mount table update behavior
dest = dest + "-new";
argv = new String[] {"-add", src, nsId, dest, "-readonly"};
- Assert.assertEquals(0, ToolRunner.run(admin, argv));
+ assertEquals(0, ToolRunner.run(admin, argv));
stateStore.loadCache(MountTableStoreImpl.class, true);
getResponse = client.getMountTableManager()
@@ -212,7 +211,7 @@ public class TestRouterAdminCLI {
@Test
public void testMountTableDefaultACL() throws Exception {
String[] argv = new String[] {"-add", "/testpath0", "ns0", "/testdir0"};
- Assert.assertEquals(0, ToolRunner.run(admin, argv));
+ assertEquals(0, ToolRunner.run(admin, argv));
stateStore.loadCache(MountTableStoreImpl.class, true);
GetMountTableEntriesRequest getRequest = GetMountTableEntriesRequest
@@ -397,6 +396,37 @@ public class TestRouterAdminCLI {
assertTrue(out.toString().contains("false"));
}
+ @Test
+ public void testCreateInvalidEntry() throws Exception {
+ String[] argv = new String[] {
+ "-add", "test-createInvalidEntry", "ns0", "/createInvalidEntry"};
+ assertEquals(-1, ToolRunner.run(admin, argv));
+
+ argv = new String[] {
+ "-add", "/test-createInvalidEntry", "ns0", "createInvalidEntry"};
+ assertEquals(-1, ToolRunner.run(admin, argv));
+
+ argv = new String[] {
+ "-add", null, "ns0", "/createInvalidEntry"};
+ assertEquals(-1, ToolRunner.run(admin, argv));
+
+ argv = new String[] {
+ "-add", "/test-createInvalidEntry", "ns0", null};
+ assertEquals(-1, ToolRunner.run(admin, argv));
+
+ argv = new String[] {
+ "-add", "", "ns0", "/createInvalidEntry"};
+ assertEquals(-1, ToolRunner.run(admin, argv));
+
+ argv = new String[] {
+ "-add", "/test-createInvalidEntry", null, "/createInvalidEntry"};
+ assertEquals(-1, ToolRunner.run(admin, argv));
+
+ argv = new String[] {
+ "-add", "/test-createInvalidEntry", "", "/createInvalidEntry"};
+ assertEquals(-1, ToolRunner.run(admin, argv));
+ }
+
/**
* Wait for the Router transforming to expected state.
* @param expectedState Expected Router state.
http://git-wip-us.apache.org/repos/asf/hadoop/blob/19292bc2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/store/records/TestMountTable.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/store/records/TestMountTable.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/store/records/TestMountTable.java
index d306f77..d5fb9ba 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/store/records/TestMountTable.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/store/records/TestMountTable.java
@@ -19,9 +19,12 @@ package org.apache.hadoop.hdfs.server.federation.store.records;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import java.io.IOException;
+import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
@@ -32,6 +35,7 @@ import org.apache.hadoop.hdfs.server.federation.resolver.RemoteLocation;
import org.apache.hadoop.hdfs.server.federation.resolver.order.DestinationOrder;
import org.apache.hadoop.hdfs.server.federation.router.RouterQuotaUsage;
import org.apache.hadoop.hdfs.server.federation.store.driver.StateStoreSerializer;
+import org.apache.hadoop.test.GenericTestUtils;
import org.junit.Test;
/**
@@ -213,4 +217,43 @@ public class TestMountTable {
assertEquals(SS_COUNT, quotaGet.getSpaceConsumed());
assertEquals(SS_QUOTA, quotaGet.getSpaceQuota());
}
+
+ @Test
+ public void testValidation() throws IOException {
+ Map<String, String> destinations = new HashMap<>();
+ destinations.put("ns0", "/testValidate-dest");
+ try {
+ MountTable.newInstance("testValidate", destinations);
+ fail("Mount table entry should be created failed.");
+ } catch (Exception e) {
+ GenericTestUtils.assertExceptionContains(
+ MountTable.ERROR_MSG_MUST_START_WITH_BACK_SLASH, e);
+ }
+
+ destinations.clear();
+ destinations.put("ns0", "testValidate-dest");
+ try {
+ MountTable.newInstance("/testValidate", destinations);
+ fail("Mount table entry should be created failed.");
+ } catch (Exception e) {
+ GenericTestUtils.assertExceptionContains(
+ MountTable.ERROR_MSG_ALL_DEST_MUST_START_WITH_BACK_SLASH, e);
+ }
+
+ destinations.clear();
+ destinations.put("", "/testValidate-dest");
+ try {
+ MountTable.newInstance("/testValidate", destinations);
+ fail("Mount table entry should be created failed.");
+ } catch (Exception e) {
+ GenericTestUtils.assertExceptionContains(
+ MountTable.ERROR_MSG_INVAILD_DEST_NS, e);
+ }
+
+ destinations.clear();
+ destinations.put("ns0", "/testValidate-dest");
+ MountTable record = MountTable.newInstance("/testValidate", destinations);
+ assertNotNull(record);
+
+ }
}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org