-
Notifications
You must be signed in to change notification settings - Fork 8.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HDFS-16724. RBF should support get the information about ancestor mount points #4719
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
/** | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you under the Apache License, Version 2.0 (the | ||
* "License"); you may not use this file except in compliance | ||
* with the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.apache.hadoop.hdfs.server.federation.router; | ||
|
||
import java.io.IOException; | ||
|
||
/** | ||
* Exception when no location found. | ||
*/ | ||
public class NoLocationException extends IOException { | ||
|
||
private static final long serialVersionUID = 1L; | ||
|
||
public NoLocationException(String message) { | ||
super(message); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -935,19 +935,22 @@ public BatchedDirectoryListing getBatchedListing(String[] srcs, | |
public HdfsFileStatus getFileInfo(String src) throws IOException { | ||
rpcServer.checkOperation(NameNode.OperationCategory.READ); | ||
|
||
final List<RemoteLocation> locations = | ||
rpcServer.getLocationsForPath(src, false, false); | ||
RemoteMethod method = new RemoteMethod("getFileInfo", | ||
new Class<?>[] {String.class}, new RemoteParam()); | ||
|
||
HdfsFileStatus ret = null; | ||
// If it's a directory, we check in all locations | ||
if (rpcServer.isPathAll(src)) { | ||
ret = getFileInfoAll(locations, method); | ||
} else { | ||
// Check for file information sequentially | ||
ret = rpcClient.invokeSequential( | ||
locations, method, HdfsFileStatus.class, null); | ||
IOException noLocationException = null; | ||
try { | ||
final List<RemoteLocation> locations = rpcServer.getLocationsForPath(src, false, false); | ||
RemoteMethod method = new RemoteMethod("getFileInfo", | ||
new Class<?>[] {String.class}, new RemoteParam()); | ||
|
||
// If it's a directory, we check in all locations | ||
if (rpcServer.isPathAll(src)) { | ||
ret = getFileInfoAll(locations, method); | ||
} else { | ||
// Check for file information sequentially | ||
ret = rpcClient.invokeSequential(locations, method, HdfsFileStatus.class, null); | ||
} | ||
} catch (NoLocationException | RouterResolveException e) { | ||
goiri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
noLocationException = e; | ||
} | ||
|
||
// If there is no real path, check mount points | ||
|
@@ -956,7 +959,7 @@ public HdfsFileStatus getFileInfo(String src) throws IOException { | |
if (children != null && !children.isEmpty()) { | ||
Map<String, Long> dates = getMountPointDates(src); | ||
long date = 0; | ||
if (dates != null && dates.containsKey(src)) { | ||
if (dates.containsKey(src)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would leave the null check; it is good practice. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Modified it since |
||
date = dates.get(src); | ||
} | ||
ret = getMountPointStatus(src, children.size(), date); | ||
|
@@ -966,6 +969,10 @@ public HdfsFileStatus getFileInfo(String src) throws IOException { | |
} | ||
} | ||
|
||
if (ret == null && noLocationException != null) { | ||
goiri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
throw noLocationException; | ||
} | ||
|
||
return ret; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1731,7 +1731,7 @@ protected List<RemoteLocation> getLocationsForPath(String path, | |
final PathLocation location = | ||
this.subclusterResolver.getDestinationForPath(path); | ||
if (location == null) { | ||
throw new IOException("Cannot find locations for " + path + " in " + | ||
throw new NoLocationException("Cannot find locations for " + path + " in " + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we have a dedicated exception, we can pass the |
||
this.subclusterResolver.getClass().getSimpleName()); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,154 @@ | ||
/** | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you under the Apache License, Version 2.0 (the | ||
* "License"); you may not use this file except in compliance | ||
* with the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.apache.hadoop.hdfs.server.federation.router; | ||
|
||
import org.apache.hadoop.conf.Configuration; | ||
import org.apache.hadoop.hdfs.protocol.ClientProtocol; | ||
import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; | ||
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; | ||
import org.apache.hadoop.hdfs.server.federation.resolver.MountTableManager; | ||
import org.apache.hadoop.hdfs.server.federation.resolver.MountTableResolver; | ||
import org.apache.hadoop.hdfs.server.federation.store.protocol.AddMountTableEntryRequest; | ||
import org.apache.hadoop.hdfs.server.federation.store.protocol.AddMountTableEntryResponse; | ||
import org.apache.hadoop.hdfs.server.federation.store.protocol.GetMountTableEntriesRequest; | ||
import org.apache.hadoop.hdfs.server.federation.store.protocol.GetMountTableEntriesResponse; | ||
import org.apache.hadoop.hdfs.server.federation.store.protocol.RemoveMountTableEntryRequest; | ||
import org.apache.hadoop.hdfs.server.federation.store.records.MountTable; | ||
import org.junit.After; | ||
import org.junit.AfterClass; | ||
import org.junit.BeforeClass; | ||
import org.junit.Test; | ||
|
||
import java.io.IOException; | ||
import java.util.Collections; | ||
|
||
import static org.junit.Assert.assertEquals; | ||
import static org.junit.Assert.assertNotNull; | ||
import static org.junit.Assert.assertTrue; | ||
import static org.junit.Assert.fail; | ||
|
||
/** | ||
* Test a router end-to-end including the MountTable without default nameservice. | ||
*/ | ||
public class TestRouterMountTableWithoutDefaultNS { | ||
private static StateStoreDFSCluster cluster; | ||
private static RouterContext routerContext; | ||
private static MountTableResolver mountTable; | ||
private static ClientProtocol routerProtocol; | ||
|
||
@BeforeClass | ||
public static void globalSetUp() throws Exception { | ||
// Build and start a federated cluster | ||
cluster = new StateStoreDFSCluster(false, 2); | ||
Configuration conf = new RouterConfigBuilder() | ||
.stateStore() | ||
.admin() | ||
.rpc() | ||
.build(); | ||
conf.setInt(RBFConfigKeys.DFS_ROUTER_ADMIN_MAX_COMPONENT_LENGTH_KEY, 20); | ||
conf.setBoolean(RBFConfigKeys.DFS_ROUTER_DEFAULT_NAMESERVICE_ENABLE, false); | ||
cluster.addRouterOverrides(conf); | ||
cluster.startCluster(); | ||
cluster.startRouters(); | ||
cluster.waitClusterUp(); | ||
|
||
// Get the end points | ||
routerContext = cluster.getRandomRouter(); | ||
Router router = routerContext.getRouter(); | ||
routerProtocol = routerContext.getClient().getNamenode(); | ||
mountTable = (MountTableResolver) router.getSubclusterResolver(); | ||
} | ||
|
||
@AfterClass | ||
public static void tearDown() { | ||
if (cluster != null) { | ||
cluster.stopRouter(routerContext); | ||
cluster.shutdown(); | ||
cluster = null; | ||
} | ||
} | ||
|
||
@After | ||
public void clearMountTable() throws IOException { | ||
RouterClient client = routerContext.getAdminClient(); | ||
MountTableManager mountTableManager = client.getMountTableManager(); | ||
GetMountTableEntriesRequest req1 = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it fit in a single line? |
||
GetMountTableEntriesRequest.newInstance("/"); | ||
GetMountTableEntriesResponse response = | ||
mountTableManager.getMountTableEntries(req1); | ||
for (MountTable entry : response.getEntries()) { | ||
RemoveMountTableEntryRequest req2 = | ||
RemoveMountTableEntryRequest.newInstance(entry.getSourcePath()); | ||
mountTableManager.removeMountTableEntry(req2); | ||
} | ||
} | ||
|
||
/** | ||
* Add a mount table entry to the mount table through the admin API. | ||
* @param entry Mount table entry to add. | ||
* @return If it was succesfully added. | ||
* @throws IOException Problems adding entries. | ||
*/ | ||
private boolean addMountTable(final MountTable entry) throws IOException { | ||
RouterClient client = routerContext.getAdminClient(); | ||
MountTableManager mountTableManager = client.getMountTableManager(); | ||
AddMountTableEntryRequest addRequest = | ||
AddMountTableEntryRequest.newInstance(entry); | ||
AddMountTableEntryResponse addResponse = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Single line |
||
mountTableManager.addMountTableEntry(addRequest); | ||
|
||
// Reload the Router cache | ||
mountTable.loadCache(true); | ||
|
||
return addResponse.getStatus(); | ||
} | ||
|
||
/** | ||
* Verify that RBF that disable default nameservice should support | ||
* get information about ancestor mount points. | ||
*/ | ||
@Test | ||
public void testGetFileInfoWithSubMountPoint() throws IOException { | ||
MountTable addEntry = MountTable.newInstance("/testdir/1", | ||
Collections.singletonMap("ns0", "/testdir/1")); | ||
assertTrue(addMountTable(addEntry)); | ||
HdfsFileStatus finfo = routerProtocol.getFileInfo("/testdir"); | ||
assertNotNull(finfo); | ||
assertEquals("supergroup", finfo.getGroup()); | ||
assertTrue(finfo.isDirectory()); | ||
} | ||
|
||
/** | ||
* Verify that RBF doesn't support get the file information | ||
* with no location and sub mount points. | ||
*/ | ||
@Test | ||
public void testGetFileInfoWithoutSubMountPoint() throws IOException { | ||
MountTable addEntry = MountTable.newInstance("/testdir/1", | ||
Collections.singletonMap("ns0", "/testdir/1")); | ||
assertTrue(addMountTable(addEntry)); | ||
try { | ||
routerProtocol.getFileInfo("/testdir2"); | ||
fail(); | ||
} catch (IOException ioe) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LambdaTestUtils#intercept There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And check for the exception type. |
||
assertTrue(ioe.getMessage().contains("Cannot find locations for /testdir2")); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a better name that reflects this is from the mount point perspective?
Extend the javadoc a little anyway.