HDFS-13226. RBF: Throw the exception if mount table entry validated failed. Contributed by maobaolong.

This commit is contained in:
Yiqun Lin 2018-03-13 11:03:31 +08:00
parent 7fab787de7
commit 19292bc264
6 changed files with 137 additions and 40 deletions

View File

@ -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

View File

@ -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;
}

View File

@ -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

View File

@ -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

View File

@ -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.

View File

@ -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);
}
}