HBASE-28775 Change the output of DatanodeInfo in the log to the hostname of the datanode #6148
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Apache9
reviewed
Aug 12, 2024
| for (int retry = 0;; retry++) { | ||
| LOG.debug("When create output stream for {}, exclude list is {}, retry={}", src, | ||
| toExcludeNodes, retry); | ||
| toExcludeNodes.stream().map(DatanodeInfo::getHostName).collect(Collectors.toSet()), retry); |
Contributor
There was a problem hiding this comment.
Let's introduce a helper method to convert DatanodeInfo to the prefer text format? I think better to include both host name and ip, and also port in the output.
And since this will be kinda expensive, we'd better put this into a LOG.isDebugEnabled if condition.
wangxin97-02
force-pushed
the
HBASE-28775
branch
from
accd55b to
44e2fd2
Compare
|
💔 -1 overall
This message was automatically generated. |
Contributor
|
Please fix the spoltess issue? |
wangxin97-02
force-pushed
the
HBASE-28775
branch
from
44e2fd2 to
7551327
Compare
Contributor
Author
It is ok. Next time, I'll pay attention. Thanks. |
|
🎊 +1 overall
This message was automatically generated. |
Apache9
reviewed
Aug 26, 2024
| } | ||
| } | ||
|
|
||
| public static String getDataNodeInfo(Set<DatanodeInfo> datanodeInfos) { |
Contributor
There was a problem hiding this comment.
Just declare the parameter as Collection, or even use Stream, so we do not lose the order when get the string description for an array?
Contributor
Author
There was a problem hiding this comment.
OK, It has been changed to Collection
|
🎊 +1 overall
This message was automatically generated. |
wangxin97-02
force-pushed
the
HBASE-28775
branch
from
7551327 to
8126cc0
Compare
|
🎊 +1 overall
This message was automatically generated. |
guluo2016
reviewed
Aug 27, 2024
...cfs/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputHelper.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
wangxin97-02
force-pushed
the
HBASE-28775
branch
from
8126cc0 to
bf3b170
Compare
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
vineet4008
approved these changes
Aug 29, 2024
guluo2016
approved these changes
Aug 31, 2024
Member
|
@1458451310 I don't have the permission either. Let's wait for Duo to have a look when he has free time. |
Contributor
I also do not have permission either. |
Apache9
reviewed
Sep 4, 2024
...cfs/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputHelper.java
Show resolved
Hide resolved
| } | ||
|
|
||
| public static String getDataNodeInfo(Collection<DatanodeInfo> datanodeInfos) { | ||
| if (datanodeInfos.isEmpty()) return "[]"; |
Contributor
There was a problem hiding this comment.
This will introduce a checkstyle error, please always use {} even if there is only a single statement.
Contributor
Author
There was a problem hiding this comment.
This will introduce a checkstyle error, please always use {} even if there is only a single statement.
It's OK
NihalJain
reviewed
Sep 4, 2024
| } | ||
|
|
||
| public static String getDataNodeInfo(Collection<DatanodeInfo> datanodeInfos) { | ||
| if (datanodeInfos.isEmpty()) return "[]"; |
Contributor
There was a problem hiding this comment.
How about we simplify this by using Collectors.joining?. For example we could do:
public static String getDataNodeInfo(Collection<DatanodeInfo> datanodeInfos) {
return datanodeInfos.stream()
.map(datanodeInfo -> new StringBuilder().append("(")
.append(datanodeInfo.getHostName()).append("/")
.append(datanodeInfo.getInfoAddr()).append(":")
.append(datanodeInfo.getInfoPort()).append(")").toString())
.collect(Collectors.joining(",", "[", "]"));
}
Also good idea to add UTs for the utility function!
Contributor
Author
There was a problem hiding this comment.
How about we simplify this by using
Collectors.joining?. For example we could do:public static String getDataNodeInfo(Collection<DatanodeInfo> datanodeInfos) { return datanodeInfos.stream() .map(datanodeInfo -> new StringBuilder().append("(") .append(datanodeInfo.getHostName()).append("/") .append(datanodeInfo.getInfoAddr()).append(":") .append(datanodeInfo.getInfoPort()).append(")").toString()) .collect(Collectors.joining(",", "[", "]")); }Also good idea to add UTs for the utility function!
This is great 👍🏻
…ame of the datanode
wangxin97-02
force-pushed
the
HBASE-28775
branch
from
bf3b170 to
64b3abc
Compare
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Apache9
approved these changes
Sep 5, 2024
NihalJain
approved these changes
Sep 5, 2024
vineet4008
approved these changes
Sep 5, 2024
Apache9
pushed a commit
that referenced
this pull request
Sep 6, 2024
…ame of the datanode (#6148) Co-authored-by: wangxin <[email protected]> Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Reviewed-by: Vineet Kumar Maheshwari <[email protected]> Reviewed-by: guluo <[email protected]> (cherry picked from commit 241bbaf)
Apache9
pushed a commit
to Apache9/hbase
that referenced
this pull request
Sep 6, 2024
…ame of the datanode (apache#6148) Co-authored-by: wangxin <[email protected]> Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Reviewed-by: Vineet Kumar Maheshwari <[email protected]> Reviewed-by: guluo <[email protected]> (cherry picked from commit 241bbaf)
Apache9
added a commit
that referenced
this pull request
Sep 15, 2024
…ame of the datanode (#6148) (#6212) Co-authored-by: wangxin <[email protected]> Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Reviewed-by: Vineet Kumar Maheshwari <[email protected]> Reviewed-by: guluo <[email protected]> (cherry picked from commit 241bbaf) Co-authored-by: WangXin <[email protected]>
Apache9
added a commit
that referenced
this pull request
Sep 15, 2024
…ame of the datanode (#6148) (#6212) Co-authored-by: wangxin <[email protected]> Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Reviewed-by: Vineet Kumar Maheshwari <[email protected]> Reviewed-by: guluo <[email protected]> (cherry picked from commit 241bbaf) Co-authored-by: WangXin <[email protected]> (cherry picked from commit f4a1904)
Apache9
added a commit
that referenced
this pull request
Sep 15, 2024
…ame of the datanode (#6148) (#6212) Co-authored-by: wangxin <[email protected]> Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Reviewed-by: Vineet Kumar Maheshwari <[email protected]> Reviewed-by: guluo <[email protected]> (cherry picked from commit 241bbaf) Co-authored-by: WangXin <[email protected]> (cherry picked from commit f4a1904)
sanjeet006py
pushed a commit
to sanjeet006py/hbase
that referenced
this pull request
Sep 26, 2025
…ame of the datanode (apache#6148) (apache#6212) Co-authored-by: wangxin <[email protected]> Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Nihal Jain <[email protected]> Reviewed-by: Vineet Kumar Maheshwari <[email protected]> Reviewed-by: guluo <[email protected]> (cherry picked from commit 241bbaf) Co-authored-by: WangXin <[email protected]> (cherry picked from commit f4a1904)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now, DatanodeInfo will be output in the print log. When we are troubleshooting and searching for slow datanode nodes, we need to convert IP addresses to hostnames, which is quite cumbersome.
I think the output log should has good readability, so it would be better to output the hostname of the datanode.