HDFS-13045. RBF: Improve error message returned from subcluster. Contributed by Inigo Goiri.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/0c93d43f Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/0c93d43f Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/0c93d43f Branch: refs/heads/HDFS-7240 Commit: 0c93d43f3d624a4fd17b3b050443d9e7e20d4f0a Parents: eefe2a1 Author: Wei Yan <w...@apache.org> Authored: Wed Apr 11 08:37:43 2018 -0700 Committer: Wei Yan <w...@apache.org> Committed: Wed Apr 11 08:37:43 2018 -0700 ---------------------------------------------------------------------- .../resolver/FederationNamespaceInfo.java | 5 ++ .../federation/resolver/MountTableResolver.java | 4 +- .../federation/resolver/RemoteLocation.java | 35 ++++++---- .../router/RemoteLocationContext.java | 7 ++ .../federation/router/RouterRpcClient.java | 67 +++++++++++++++++++- .../federation/store/records/MountTable.java | 2 +- .../store/records/impl/pb/MountTablePBImpl.java | 2 +- .../hdfs/server/federation/MockResolver.java | 12 +++- .../federation/router/TestRouterAdmin.java | 4 +- .../server/federation/router/TestRouterRpc.java | 48 ++++++++++++++ .../store/records/TestMountTable.java | 4 +- 11 files changed, 165 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/0c93d43f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/FederationNamespaceInfo.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/FederationNamespaceInfo.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/FederationNamespaceInfo.java index edcd308..33edd30 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/FederationNamespaceInfo.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/FederationNamespaceInfo.java @@ -48,6 +48,11 @@ public class FederationNamespaceInfo extends RemoteLocationContext { return this.nameserviceId; } + @Override + public String getSrc() { + return null; + } + /** * The HDFS cluster id for this namespace. * http://git-wip-us.apache.org/repos/asf/hadoop/blob/0c93d43f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java index 9713138..3f6efd6 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java @@ -388,7 +388,7 @@ public class MountTableResolver } else { // Not found, use default location RemoteLocation remoteLocation = - new RemoteLocation(defaultNameService, path); + new RemoteLocation(defaultNameService, path, path); List<RemoteLocation> locations = Collections.singletonList(remoteLocation); ret = new PathLocation(null, locations); @@ -519,7 +519,7 @@ public class MountTableResolver newPath += Path.SEPARATOR; } newPath += remainingPath; - RemoteLocation remoteLocation = new RemoteLocation(nsId, newPath); + RemoteLocation remoteLocation = new RemoteLocation(nsId, newPath, path); locations.add(remoteLocation); } DestinationOrder order = entry.getDestOrder(); http://git-wip-us.apache.org/repos/asf/hadoop/blob/0c93d43f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/RemoteLocation.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/RemoteLocation.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/RemoteLocation.java index 6aa12ce..77d0500 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/RemoteLocation.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/RemoteLocation.java @@ -20,8 +20,8 @@ package org.apache.hadoop.hdfs.server.federation.resolver; import org.apache.hadoop.hdfs.server.federation.router.RemoteLocationContext; /** - * A single in a remote namespace consisting of a nameservice ID - * and a HDFS path. + * A location in a remote namespace consisting of a nameservice ID and a HDFS + * path (destination). It also contains the federated location (source). */ public class RemoteLocation extends RemoteLocationContext { @@ -30,16 +30,19 @@ public class RemoteLocation extends RemoteLocationContext { /** Identifier of the namenode in the namespace for this location. */ private final String namenodeId; /** Path in the remote location. */ - private final String path; + private final String dstPath; + /** Original path in federation. */ + private final String srcPath; /** * Create a new remote location. * - * @param nsId - * @param pPath + * @param nsId Destination namespace. + * @param dPath Path in the destination namespace. + * @param sPath Path in the federated level. */ - public RemoteLocation(String nsId, String pPath) { - this(nsId, null, pPath); + public RemoteLocation(String nsId, String dPath, String sPath) { + this(nsId, null, dPath, sPath); } /** @@ -47,12 +50,15 @@ public class RemoteLocation extends RemoteLocationContext { * namespace. * * @param nsId Destination namespace. - * @param pPath Path in the destination namespace. + * @param nnId Destination namenode. + * @param dPath Path in the destination namespace. + * @param sPath Path in the federated level */ - public RemoteLocation(String nsId, String nnId, String pPath) { + public RemoteLocation(String nsId, String nnId, String dPath, String sPath) { this.nameserviceId = nsId; this.namenodeId = nnId; - this.path = pPath; + this.dstPath = dPath; + this.srcPath = sPath; } @Override @@ -66,11 +72,16 @@ public class RemoteLocation extends RemoteLocationContext { @Override public String getDest() { - return this.path; + return this.dstPath; + } + + @Override + public String getSrc() { + return this.srcPath; } @Override public String toString() { - return getNameserviceId() + "->" + this.path; + return getNameserviceId() + "->" + this.dstPath; } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/hadoop/blob/0c93d43f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RemoteLocationContext.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RemoteLocationContext.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RemoteLocationContext.java index a90c460..0959eaa 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RemoteLocationContext.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RemoteLocationContext.java @@ -39,6 +39,13 @@ public abstract class RemoteLocationContext */ public abstract String getDest(); + /** + * Original source location. + * + * @return Source path. + */ + public abstract String getSrc(); + @Override public int hashCode() { return new HashCodeBuilder(17, 31) http://git-wip-us.apache.org/repos/asf/hadoop/blob/0c93d43f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java index e2c9cb4..6c657b2 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hdfs.server.federation.router; +import java.io.FileNotFoundException; import java.io.IOException; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; @@ -62,6 +63,7 @@ import org.apache.hadoop.security.UserGroupInformation; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.ThreadFactoryBuilder; /** @@ -611,7 +613,7 @@ public class RouterRpcClient { UserGroupInformation ugi = RouterRpcServer.getRemoteUser(); List<? extends FederationNamenodeContext> nns = getNamenodesForNameservice(nsId); - RemoteLocationContext loc = new RemoteLocation(nsId, "/"); + RemoteLocationContext loc = new RemoteLocation(nsId, "/", "/"); Class<?> proto = method.getProtocol(); Method m = method.getMethod(); Object[] params = method.getParams(loc); @@ -727,8 +729,12 @@ public class RouterRpcClient { firstResult = result; } } catch (IOException ioe) { + // Localize the exception + + ioe = processException(ioe, loc); + // Record it and move on - lastThrownException = (IOException) ioe; + lastThrownException = ioe; if (firstThrownException == null) { firstThrownException = lastThrownException; } @@ -757,6 +763,63 @@ public class RouterRpcClient { } /** + * Exception messages might contain local subcluster paths. This method + * generates a new exception with the proper message. + * @param ioe Original IOException. + * @param loc Location we are processing. + * @return Exception processed for federation. + */ + private IOException processException( + IOException ioe, RemoteLocationContext loc) { + + if (ioe instanceof RemoteException) { + RemoteException re = (RemoteException)ioe; + String newMsg = processExceptionMsg( + re.getMessage(), loc.getDest(), loc.getSrc()); + RemoteException newException = + new RemoteException(re.getClassName(), newMsg); + newException.setStackTrace(ioe.getStackTrace()); + return newException; + } + + if (ioe instanceof FileNotFoundException) { + String newMsg = processExceptionMsg( + ioe.getMessage(), loc.getDest(), loc.getSrc()); + FileNotFoundException newException = new FileNotFoundException(newMsg); + newException.setStackTrace(ioe.getStackTrace()); + return newException; + } + + return ioe; + } + + /** + * Process a subcluster message and make it federated. + * @param msg Original exception message. + * @param dst Path in federation. + * @param src Path in the subcluster. + * @return Message processed for federation. + */ + @VisibleForTesting + static String processExceptionMsg( + final String msg, final String dst, final String src) { + if (dst.equals(src) || !dst.startsWith("/") || !src.startsWith("/")) { + return msg; + } + + String newMsg = msg.replaceFirst(dst, src); + int minLen = Math.min(dst.length(), src.length()); + for (int i = 0; newMsg.equals(msg) && i < minLen; i++) { + // Check if we can replace sub folders + String dst1 = dst.substring(0, dst.length() - 1 - i); + String src1 = src.substring(0, src.length() - 1 - i); + newMsg = msg.replaceFirst(dst1, src1); + } + + return newMsg; + } + + /** * Checks if a result matches the required result class. * * @param expectedClass Required result class, null to skip the check. http://git-wip-us.apache.org/repos/asf/hadoop/blob/0c93d43f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java index 60496ef..005882e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java @@ -134,7 +134,7 @@ public abstract class MountTable extends BaseRecord { for (Entry<String, String> entry : destinations.entrySet()) { String nsId = entry.getKey(); String path = normalizeFileSystemPath(entry.getValue()); - RemoteLocation location = new RemoteLocation(nsId, path); + RemoteLocation location = new RemoteLocation(nsId, path, src); locations.add(location); } http://git-wip-us.apache.org/repos/asf/hadoop/blob/0c93d43f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/impl/pb/MountTablePBImpl.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/impl/pb/MountTablePBImpl.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/impl/pb/MountTablePBImpl.java index 48f93bc..e62d0a8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/impl/pb/MountTablePBImpl.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/impl/pb/MountTablePBImpl.java @@ -102,7 +102,7 @@ public class MountTablePBImpl extends MountTable implements PBRecord { for (RemoteLocationProto dest : destList) { String nsId = dest.getNameserviceId(); String path = dest.getPath(); - RemoteLocation loc = new RemoteLocation(nsId, path); + RemoteLocation loc = new RemoteLocation(nsId, path, getSourcePath()); ret.add(loc); } return ret; http://git-wip-us.apache.org/repos/asf/hadoop/blob/0c93d43f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/MockResolver.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/MockResolver.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/MockResolver.java index 151d731..0ce0944 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/MockResolver.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/MockResolver.java @@ -80,7 +80,8 @@ public class MockResolver this.locations.put(mount, locationsList); } - final RemoteLocation remoteLocation = new RemoteLocation(nsId, location); + final RemoteLocation remoteLocation = + new RemoteLocation(nsId, location, mount); if (!locationsList.contains(remoteLocation)) { locationsList.add(remoteLocation); } @@ -270,10 +271,15 @@ public class MockResolver for (String key : keys) { if (path.startsWith(key)) { for (RemoteLocation location : this.locations.get(key)) { - String finalPath = location.getDest() + path.substring(key.length()); + String finalPath = location.getDest(); + String extraPath = path.substring(key.length()); + if (finalPath.endsWith("/") && extraPath.startsWith("/")) { + extraPath = extraPath.substring(1); + } + finalPath += extraPath; String nameservice = location.getNameserviceId(); RemoteLocation remoteLocation = - new RemoteLocation(nameservice, finalPath); + new RemoteLocation(nameservice, finalPath, path); remoteLocations.add(remoteLocation); } break; http://git-wip-us.apache.org/repos/asf/hadoop/blob/0c93d43f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdmin.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdmin.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdmin.java index 3c1a136..10b71d7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdmin.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdmin.java @@ -251,7 +251,7 @@ public class TestRouterAdmin { // Verify starting condition MountTable entry = getMountTableEntry("/"); assertEquals( - Collections.singletonList(new RemoteLocation("ns0", "/")), + Collections.singletonList(new RemoteLocation("ns0", "/", "/")), entry.getDestinations()); // Edit the entry for / @@ -264,7 +264,7 @@ public class TestRouterAdmin { // Verify edited condition entry = getMountTableEntry("/"); assertEquals( - Collections.singletonList(new RemoteLocation("ns1", "/")), + Collections.singletonList(new RemoteLocation("ns1", "/", "/")), entry.getDestinations()); } http://git-wip-us.apache.org/repos/asf/hadoop/blob/0c93d43f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java index 414e112..d5ec093 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java @@ -24,6 +24,7 @@ import static org.apache.hadoop.hdfs.server.federation.FederationTestUtils.delet import static org.apache.hadoop.hdfs.server.federation.FederationTestUtils.getFileStatus; import static org.apache.hadoop.hdfs.server.federation.FederationTestUtils.verifyFileExists; import static org.apache.hadoop.hdfs.server.federation.MiniRouterDFSCluster.TEST_STRING; +import static org.apache.hadoop.test.GenericTestUtils.assertExceptionContains; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -31,6 +32,7 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.IOException; import java.lang.reflect.Method; @@ -1023,6 +1025,52 @@ public class TestRouterRpc { } @Test + public void testProxyExceptionMessages() throws IOException { + + // Install a mount point to a different path to check + MockResolver resolver = + (MockResolver)router.getRouter().getSubclusterResolver(); + String ns0 = cluster.getNameservices().get(0); + resolver.addLocation("/mnt", ns0, "/"); + + try { + FsPermission permission = new FsPermission("777"); + routerProtocol.mkdirs("/mnt/folder0/folder1", permission, false); + fail("mkdirs for non-existing parent folder should have failed"); + } catch (IOException ioe) { + assertExceptionContains("/mnt/folder0", ioe, + "Wrong path in exception for mkdirs"); + } + + try { + FsPermission permission = new FsPermission("777"); + routerProtocol.setPermission("/mnt/testfile.txt", permission); + fail("setPermission for non-existing file should have failed"); + } catch (IOException ioe) { + assertExceptionContains("/mnt/testfile.txt", ioe, + "Wrong path in exception for setPermission"); + } + + try { + FsPermission permission = new FsPermission("777"); + routerProtocol.mkdirs("/mnt/folder0/folder1", permission, false); + routerProtocol.delete("/mnt/folder0", false); + fail("delete for non-existing file should have failed"); + } catch (IOException ioe) { + assertExceptionContains("/mnt/folder0", ioe, + "Wrong path in exception for delete"); + } + + resolver.cleanRegistrations(); + + // Check corner cases + assertEquals( + "Parent directory doesn't exist: /ns1/a/a/b", + RouterRpcClient.processExceptionMsg( + "Parent directory doesn't exist: /a/a/b", "/a", "/ns1/a")); + } + + @Test public void testErasureCoding() throws IOException { LOG.info("List the available erasurce coding policies"); http://git-wip-us.apache.org/repos/asf/hadoop/blob/0c93d43f/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/records/TestMountTable.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/records/TestMountTable.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/records/TestMountTable.java index d5fb9ba..43cf176 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/records/TestMountTable.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/store/records/TestMountTable.java @@ -50,8 +50,8 @@ public class TestMountTable { private static final String DST_PATH_1 = "/path/path2"; private static final List<RemoteLocation> DST = new LinkedList<>(); static { - DST.add(new RemoteLocation(DST_NS_0, DST_PATH_0)); - DST.add(new RemoteLocation(DST_NS_1, DST_PATH_1)); + DST.add(new RemoteLocation(DST_NS_0, DST_PATH_0, SRC)); + DST.add(new RemoteLocation(DST_NS_1, DST_PATH_1, SRC)); } private static final Map<String, String> DST_MAP = new LinkedHashMap<>(); static { --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
