Skip to content

HDFS-17181 WebHDFS: Route all CREATE requests to the BlockManager #6108

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 20, 2024
Merged

HDFS-17181 WebHDFS: Route all CREATE requests to the BlockManager #6108

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
20dda3b
Fixes HDFS-17181 by routing all CREATE requests to the BlockManager
lfrancke Sep 22, 2023
f0c73b2
Merge branch 'trunk' into fix/HDFS-17181
lfrancke Feb 14, 2024
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
52 changes: 29 additions & 23 deletions ...ain/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;

import javax.servlet.ServletContext;
Expand Down Expand Up @@ -278,6 +279,9 @@ protected void queueExternalCall(ExternalCall call)
namenode.queueExternalCall(call);
}

/**
* Chooses a Datanode to redirect a request to.
*/
@VisibleForTesting
static DatanodeInfo chooseDatanode(final NameNode namenode,
final String path, final HttpOpParam.Op op, final long openOffset,
Expand All @@ -288,18 +292,18 @@ static DatanodeInfo chooseDatanode(final NameNode namenode,
throw new IOException("Namesystem has not been initialized yet.");
}
final BlockManager bm = fsn.getBlockManager();
HashSet<Node> excludes = new HashSet<Node>();

Set<Node> excludes = new HashSet<>();
Copy link
Member Author

Choose a reason for hiding this comment

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

This just removes a warning

if (excludeDatanodes != null) {
for (String host : StringUtils
.getTrimmedStringCollection(excludeDatanodes)) {
int idx = host.indexOf(":");
int idx = host.indexOf(':');
Node excludeNode = null;
if (idx != -1) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This makes the code easier to read by just switching the != with the == condition. No other code changes

excludeNode = bm.getDatanodeManager().getDatanodeByXferAddr(
host.substring(0, idx), Integer.parseInt(host.substring(idx + 1)));
} else {
if (idx == -1) {
excludeNode = bm.getDatanodeManager().getDatanodeByHost(host);
} else {
excludeNode = bm.getDatanodeManager().getDatanodeByXferAddr(
host.substring(0, idx), Integer.parseInt(host.substring(idx + 1)));
}

if (excludeNode != null) {
Expand All @@ -311,25 +315,15 @@ static DatanodeInfo chooseDatanode(final NameNode namenode,
}
}

if (op == PutOpParam.Op.CREATE) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the actual change. It used to treat CREATE special by calling the BlockManager.
This removes the special case and instead makes calling the BlockManager the norm for all requests except OPEN, APPEND and GETFILECHECKSUM for which it will pick an actual replica.

//choose a datanode near to client
final DatanodeDescriptor clientNode = bm.getDatanodeManager(
).getDatanodeByHost(remoteAddr);
if (clientNode != null) {
final DatanodeStorageInfo[] storages = bm.chooseTarget4WebHDFS(
path, clientNode, excludes, blocksize);
if (storages.length > 0) {
return storages[0].getDatanodeDescriptor();
}
}
} else if (op == GetOpParam.Op.OPEN
// For these operations choose a datanode containing a replica
if (op == GetOpParam.Op.OPEN
|| op == GetOpParam.Op.GETFILECHECKSUM
|| op == PostOpParam.Op.APPEND) {
//choose a datanode containing a replica
final NamenodeProtocols np = getRPCServer(namenode);
if (status == null) {
throw new FileNotFoundException("File " + path + " not found.");
}

final long len = status.getLen();
if (op == GetOpParam.Op.OPEN) {
if (openOffset < 0L || (openOffset >= len && len > 0)) {
Expand All @@ -344,10 +338,22 @@ static DatanodeInfo chooseDatanode(final NameNode namenode,
final int count = locations.locatedBlockCount();
if (count > 0) {
return bestNode(locations.get(0).getLocations(), excludes);
} else {
throw new IOException("Block could not be located. Path=" + path + ", offset=" + offset);
}
}
}

// All other operations don't affect a specific node so let the BlockManager pick a target
DatanodeDescriptor clientNode = bm.getDatanodeManager(
).getDatanodeByHost(remoteAddr);

DatanodeStorageInfo[] storages = bm.chooseTarget4WebHDFS(
path, clientNode, excludes, blocksize);
if (storages.length > 0) {
return storages[0].getDatanodeDescriptor();
}

return (DatanodeDescriptor)bm.getDatanodeManager().getNetworkTopology(
).chooseRandom(NodeBase.ROOT, excludes);
}
Expand All @@ -358,13 +364,13 @@ static DatanodeInfo chooseDatanode(final NameNode namenode,
* to return the first element of the node here.
*/
protected static DatanodeInfo bestNode(DatanodeInfo[] nodes,
HashSet<Node> excludes) throws IOException {
Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated: Just removes warnings again and makes code clearer

Set<Node> excludes) throws IOException {
for (DatanodeInfo dn: nodes) {
if (false == dn.isDecommissioned() && false == excludes.contains(dn)) {
if (!dn.isDecommissioned() && !excludes.contains(dn)) {
return dn;
}
}
throw new IOException("No active nodes contain this block");
throw new IOException("No active and not excluded nodes contain this block");
}

public long renewDelegationToken(Token<DelegationTokenIdentifier> token)
Expand Down