[
https://issues.apache.org/jira/browse/HDFS-17181?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17817316#comment-17817316
]
ASF GitHub Bot commented on HDFS-17181:
---------------------------------------
lfrancke commented on code in PR #6108:
URL: https://github.com/apache/hadoop/pull/6108#discussion_r1489245114
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java:
##########
@@ -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 {
Review Comment:
Unrelated: Just removes warnings again and makes code clearer
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java:
##########
@@ -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<>();
if (excludeDatanodes != null) {
for (String host : StringUtils
.getTrimmedStringCollection(excludeDatanodes)) {
- int idx = host.indexOf(":");
+ int idx = host.indexOf(':');
Node excludeNode = null;
- if (idx != -1) {
Review Comment:
This makes the code easier to read by just switching the `!=` with the `==`
condition. No other code changes
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java:
##########
@@ -311,25 +315,15 @@ static DatanodeInfo chooseDatanode(final NameNode
namenode,
}
}
- if (op == PutOpParam.Op.CREATE) {
Review Comment:
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.
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java:
##########
@@ -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<>();
Review Comment:
This just removes a warning
> WebHDFS not considering whether a DN is good when called from outside the
> cluster
> ---------------------------------------------------------------------------------
>
> Key: HDFS-17181
> URL: https://issues.apache.org/jira/browse/HDFS-17181
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: namenode, webhdfs
> Affects Versions: 3.3.6
> Reporter: Lars Francke
> Priority: Major
> Labels: pull-request-available
> Attachments: Test_fix_for_HDFS-171811.patch
>
>
> When calling WebHDFS to create a file (I'm sure the same problem occurs for
> other actions e.g. OPEN but I haven't checked all of them yet) it will
> happily redirect to nodes that are in maintenance.
> The reason is in the
> [{{chooseDatanode}}|https://github.com/apache/hadoop/blob/rel/release-3.3.6/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java#L307C9-L315]
> method in {{NamenodeWebHdfsMethods}} where it will only call the
> {{BlockPlacementPolicy}} (which considers all these edge cases) in case the
> {{remoteAddr}} (i.e. the address making the request to WebHDFS) is also
> running a DataNode.
>
> In all other cases it just refers to
> [{{NetworkTopology#chooseRandom}}|https://github.com/apache/hadoop/blob/rel/release-3.3.6/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java#L342-L343]
> which does not consider any of these circumstances (e.g. load, maintenance).
> I don't understand the reason to not just always refer to the placement
> policy and we're currently testing a patch to do just that.
> I have attached a draft patch for now.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
