From 4813a37023a80e927532857de65fcfdd0e25278c Mon Sep 17 00:00:00 2001 From: Ayush Saxena Date: Thu, 20 Aug 2020 23:54:35 +0530 Subject: [PATCH] HDFS-15535. RBF: Fix Namespace path to snapshot path resolution for snapshot API. Contributed by Ayush Saxena. --- .../federation/router/RouterRpcClient.java | 50 ++++++++++++++++--- .../federation/router/RouterSnapshot.java | 18 ++++--- ...MultipleDestinationMountTableResolver.java | 33 ++++++++++++ 3 files changed, 88 insertions(+), 13 deletions(-) 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 dae4b93564..fb068bfc33 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 @@ -849,6 +849,45 @@ public T invokeSequential( final List locations, final RemoteMethod remoteMethod, Class expectedResultClass, Object expectedResultValue) throws IOException { + return (T) invokeSequential(remoteMethod, locations, expectedResultClass, + expectedResultValue).getResult(); + } + + /** + * Invokes sequential proxy calls to different locations. Continues to invoke + * calls until the success condition is met, or until all locations have been + * attempted. + * + * The success condition may be specified by: + *
    + *
  • An expected result class + *
  • An expected result value + *
+ * + * If no expected result class/values are specified, the success condition is + * a call that does not throw a remote exception. + * + * This returns RemoteResult, which contains the invoked location as well + * as the result. + * + * @param The type of the remote location. + * @param The type of the remote method return. + * @param remoteMethod The remote method and parameters to invoke. + * @param locations List of locations/nameservices to call concurrently. + * @param expectedResultClass In order to be considered a positive result, the + * return type must be of this class. + * @param expectedResultValue In order to be considered a positive result, the + * return value must equal the value of this object. + * @return The result of the first successful call, or if no calls are + * successful, the result of the first RPC call executed, along with + * the invoked location in form of RemoteResult. + * @throws IOException if the success condition is not met, return the first + * remote exception generated. + */ + public RemoteResult invokeSequential( + final RemoteMethod remoteMethod, final List locations, + Class expectedResultClass, Object expectedResultValue) + throws IOException { final UserGroupInformation ugi = RouterRpcServer.getRemoteUser(); final Method m = remoteMethod.getMethod(); @@ -867,9 +906,9 @@ public T invokeSequential( if (isExpectedClass(expectedResultClass, result) && isExpectedValue(expectedResultValue, result)) { // Valid result, stop here - @SuppressWarnings("unchecked") - T ret = (T)result; - return ret; + @SuppressWarnings("unchecked") R location = (R) loc; + @SuppressWarnings("unchecked") T ret = (T) result; + return new RemoteResult<>(location, ret); } if (firstResult == null) { firstResult = result; @@ -907,9 +946,8 @@ public T invokeSequential( throw thrownExceptions.get(0); } // Return the first result, whether it is the value or not - @SuppressWarnings("unchecked") - T ret = (T)firstResult; - return ret; + @SuppressWarnings("unchecked") T ret = (T) firstResult; + return new RemoteResult<>(locations.get(0), ret); } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterSnapshot.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterSnapshot.java index 3f4f4cb46b..9a47f2a012 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterSnapshot.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterSnapshot.java @@ -104,10 +104,12 @@ public String createSnapshot(String snapshotRoot, String snapshotName) result = firstelement.getValue(); result = result.replaceFirst(loc.getDest(), loc.getSrc()); } else { - result = rpcClient.invokeSequential( - locations, method, String.class, null); - RemoteLocation loc = locations.get(0); - result = result.replaceFirst(loc.getDest(), loc.getSrc()); + RemoteResult response = + rpcClient.invokeSequential(method, locations, String.class, null); + RemoteLocation loc = response.getLocation(); + String invokedResult = response.getResult(); + result = invokedResult + .replaceFirst(loc.getDest(), loc.getSrc()); } return result; } @@ -180,9 +182,11 @@ public SnapshotStatus[] getSnapshotListing(String snapshotRoot) s.setParentFullPath(DFSUtil.string2Bytes(mountPath)); } } else { - response = rpcClient.invokeSequential( - locations, remoteMethod, SnapshotStatus[].class, null); - RemoteLocation loc = locations.get(0); + RemoteResult invokedResponse = rpcClient + .invokeSequential(remoteMethod, locations, SnapshotStatus[].class, + null); + RemoteLocation loc = invokedResponse.getLocation(); + response = invokedResponse.getResult(); for (SnapshotStatus s : response) { String mountPath = DFSUtil.bytes2String(s.getParentFullPath()). replaceFirst(loc.getDest(), loc.getSrc()); diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRPCMultipleDestinationMountTableResolver.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRPCMultipleDestinationMountTableResolver.java index d00b93c430..181442d647 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRPCMultipleDestinationMountTableResolver.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRPCMultipleDestinationMountTableResolver.java @@ -43,6 +43,7 @@ import org.apache.hadoop.hdfs.DFSConfigKeys; import org.apache.hadoop.hdfs.DFSTestUtil; import org.apache.hadoop.hdfs.DistributedFileSystem; +import org.apache.hadoop.hdfs.protocol.SnapshotStatus; import org.apache.hadoop.hdfs.server.federation.MiniRouterDFSCluster.RouterContext; import org.apache.hadoop.hdfs.server.federation.RouterConfigBuilder; import org.apache.hadoop.hdfs.server.federation.StateStoreDFSCluster; @@ -492,6 +493,38 @@ public void testIsMultiDestDir() throws Exception { assertFalse(client.isMultiDestDirectory("/mount/dir")); } + /** + * Verifies the snapshot location returned after snapshot operations is in + * accordance to the mount path. + */ + @Test + public void testSnapshotPathResolution() throws Exception { + // Create a mount entry with non isPathAll order, so as to call + // invokeSequential. + Map destMap = new HashMap<>(); + destMap.put("ns0", "/tmp_ns0"); + destMap.put("ns1", "/tmp_ns1"); + nnFs0.mkdirs(new Path("/tmp_ns0")); + nnFs1.mkdirs(new Path("/tmp_ns1")); + MountTable addEntry = MountTable.newInstance("/mountSnap", destMap); + addEntry.setDestOrder(DestinationOrder.HASH); + assertTrue(addMountTable(addEntry)); + // Create the actual directory in the destination second in sequence of + // invokeSequential. + nnFs0.mkdirs(new Path("/tmp_ns0/snapDir")); + Path snapDir = new Path("/mountSnap/snapDir"); + Path snapshotPath = new Path("/mountSnap/snapDir/.snapshot/snap"); + routerFs.allowSnapshot(snapDir); + // Verify the snapshot path returned after createSnapshot is as per mount + // path. + Path snapshot = routerFs.createSnapshot(snapDir, "snap"); + assertEquals(snapshotPath, snapshot); + // Verify the snapshot path returned as part of snapshotListing is as per + // mount path. + SnapshotStatus[] snapshots = routerFs.getSnapshotListing(snapDir); + assertEquals(snapshotPath, snapshots[0].getFullPath()); + } + @Test public void testRenameMultipleDestDirectories() throws Exception { // Test renaming directories using rename API.