ayushtkn commented on a change in pull request #2166:
URL: https://github.com/apache/hadoop/pull/2166#discussion_r461350292
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterSnapshot.java
##########
@@ -166,14 +167,30 @@ public void renameSnapshot(String snapshotRoot, String
oldSnapshotName,
RemoteMethod remoteMethod = new RemoteMethod("getSnapshotListing",
new Class<?>[]{String.class},
new RemoteParam());
+ SnapshotStatus[] response;
if (rpcServer.isInvokeConcurrent(snapshotRoot)) {
Map<RemoteLocation, SnapshotStatus[]> ret = rpcClient.invokeConcurrent(
locations, remoteMethod, true, false, SnapshotStatus[].class);
- return ret.values().iterator().next();
+ response = ret.values().iterator().next();
+ String src = ret.keySet().iterator().next().getSrc();
+ String dst = ret.keySet().iterator().next().getDest();
+ for (SnapshotStatus s : response) {
+ String mountPath =
+ new String(s.getParentFullPath()).replaceFirst(src, dst);
+ s.setParentFullPath(mountPath.getBytes());
+ }
} else {
- return rpcClient.invokeSequential(
+ response = rpcClient.invokeSequential(
locations, remoteMethod, SnapshotStatus[].class, null);
+ RemoteLocation loc = locations.get(0);
Review comment:
The actual location won't be always the first one in the list.
`RouterRpcClient#L858` iterates over the list and triggers call to them
sequentially, if the first location doesn't give the specific response, then
second and so on. Adding a line at `RouterRpcClient` at `L872`, shall make this
logic work -
`` Collections.swap(locations, 0, locations.indexOf(loc));``
Apart everything seems good here.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterSnapshot.java
##########
@@ -166,14 +167,30 @@ public void renameSnapshot(String snapshotRoot, String
oldSnapshotName,
RemoteMethod remoteMethod = new RemoteMethod("getSnapshotListing",
new Class<?>[]{String.class},
new RemoteParam());
+ SnapshotStatus[] response;
if (rpcServer.isInvokeConcurrent(snapshotRoot)) {
Map<RemoteLocation, SnapshotStatus[]> ret = rpcClient.invokeConcurrent(
locations, remoteMethod, true, false, SnapshotStatus[].class);
- return ret.values().iterator().next();
+ response = ret.values().iterator().next();
+ String src = ret.keySet().iterator().next().getSrc();
+ String dst = ret.keySet().iterator().next().getDest();
+ for (SnapshotStatus s : response) {
+ String mountPath =
+ new String(s.getParentFullPath()).replaceFirst(src, dst);
+ s.setParentFullPath(mountPath.getBytes());
+ }
} else {
- return rpcClient.invokeSequential(
+ response = rpcClient.invokeSequential(
locations, remoteMethod, SnapshotStatus[].class, null);
+ RemoteLocation loc = locations.get(0);
Review comment:
The actual location won't be always the first one in the list.
`RouterRpcClient#L858` iterates over the list and triggers call to them
sequentially, if the first location doesn't give the specific response, then
second and so on. Adding a line at `RouterRpcClient` at `L872` just before ``
return ret;``, shall make this logic work -
`` Collections.swap(locations, 0, locations.indexOf(loc));``
Apart everything seems good here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
