-
Notifications
You must be signed in to change notification settings - Fork 9.1k
MAPREDUCE-7429 Application log link is not accessible via TimelineServer WebUI in IPV6 scenario #5236
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
base: trunk
Are you sure you want to change the base?
Conversation
6028423 to
b1a0e4f
Compare
...op-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
b1a0e4f to
2bfbf9c
Compare
|
@Daniel-009497 Thank you very much for your contribution, but we need to fix checkstyle issue. Don't worry about javadoc issue, it seems to be a problem with jdk. |
2bfbf9c to
e156724
Compare
|
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
e156724 to
e8eba06
Compare
|
💔 -1 overall
This message was automatically generated. |
| } | ||
|
|
||
| public static String normalizeV6Address(String target) { | ||
| if (!target.startsWith("[")) { |
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.
I'm pretty sure regex would help here.
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.
No?
| target = target.trim(); | ||
| String port = target.substring(target.lastIndexOf(":") + 1); | ||
| String addr = target.substring(0, i); | ||
| target = "[" + addr + "]" + ":" + port; |
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.
Make the input target final in the declaration and return a new variable.
public static String normalizeV6Address(final String target) {
| return createSocketAddrForHost(host, port); | ||
| } | ||
|
|
||
| public static String normalizeV6Address(String target) { |
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.
Add unit tests for this.
|
|
||
| public static URI constructResURI(Configuration conf, String address, | ||
| String uri) { | ||
| if (StringUtils.countMatches(address, ":") > 2) { |
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.
This probable be in NetUtils and be a proper isIPV6Address() or something.
e8eba06 to
e5ec449
Compare
| if (StringUtils.countMatches(target, ":") > 2) { | ||
| // if scheme exists in the target, for example: | ||
| // https://ffff:ffff:ffff:ffff::1:XXXX will be formed like | ||
| // https://[ffff:ffff:ffff:ffff::1]:XXXX |
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.
The code looks fine, we'd better use a specific port in the comments, such as 8088
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.
Thanks for review.
Just modified as you mentioned.
Pls help to merge once the pipeline is done.
And pls help to review the other PR I raised if you got time.
Thanks a lot.
bd10b58 to
56eb9ec
Compare
|
@Daniel-009497 Thank you very much for your contribution, but we need to wait for |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetUtils.java
Outdated
Show resolved
Hide resolved
...op-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java
Outdated
Show resolved
Hide resolved
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java
Outdated
Show resolved
Hide resolved
56eb9ec to
07973a0
Compare
|
💔 -1 overall
This message was automatically generated. |
…ver WebUI in IPV6 scenario
07973a0 to
85aa877
Compare
|
@goiri |
|
I found that this change is in the yarn module, but the title is mapreduce. waiting yetus compilation result If our changes affect the mapreduce module, we'd better wait for MAPREDUCE-7428 (#5243), wait for this to complete, and do compilation again. |
|
💔 -1 overall
This message was automatically generated. |
| } | ||
|
|
||
| public static String normalizeV6Address(String target) { | ||
| if (!target.startsWith("[")) { |
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.
No?
| } | ||
| target = target.trim(); | ||
| boolean hasScheme = target.contains("://"); | ||
| if (NetUtils.isIPV6Address(target)) { |
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.

Application log link is not accessible via Timelineserver Web UI under IPV6 scenario owing to no ipv6 normalization for IP address.