Skip to content
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

Merged
merged 3 commits into from
Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
Copy link
Member

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.

*/
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
Expand Up @@ -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
Expand All @@ -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)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave the null check; it is good practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified it since getMountPointDates always return a not null response. I will roll back this modification.

date = dates.get(src);
}
ret = getMountPointStatus(src, children.size(), date);
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we have a dedicated exception, we can pass the path and class parameters and build the message in NoLocationException

this.subclusterResolver.getClass().getSimpleName());
}

Expand Down
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 =
Copy link
Member

Choose a reason for hiding this comment

The 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 =
Copy link
Member

Choose a reason for hiding this comment

The 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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LambdaTestUtils#intercept

Copy link
Member

Choose a reason for hiding this comment

The 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"));
}
}
}