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/HDFS-12996 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
