You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/06/09 16:42:18 UTC
[GitHub] [hadoop-ozone] runzhiwang opened a new pull request #1044: HDDS-3743. Avoid NetUtils.normalize when get DatanodeDetails from proto
runzhiwang opened a new pull request #1044:
URL: https://github.com/apache/hadoop-ozone/pull/1044
## What changes were proposed in this pull request?
NetUtils.normalize cost 0.17%, because it needs to do Matcher.replaceAll. It happen in the critical path, i.e. lookUpKey when read. we need not to do this when getFromProtoBuf, So we can avoid normalize by check the last character, if the last character is not `/`, we need not normalize.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3743
## How was this patch tested?
Existed tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on pull request #1044: HDDS-3743. Avoid NetUtils#normalize when get DatanodeDetails from proto
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on pull request #1044:
URL: https://github.com/apache/hadoop-ozone/pull/1044#issuecomment-646422305
Thanks @runzhiwang for the contribution. I've merged the patch to master.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] codecov-commenter commented on pull request #1044: HDDS-3743. Avoid NetUtils#normalize when get DatanodeDetails from proto
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1044:
URL: https://github.com/apache/hadoop-ozone/pull/1044#issuecomment-643731591
# [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1044?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@9798cdb`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hadoop-ozone/pull/1044?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1044 +/- ##
=========================================
Coverage ? 69.41%
Complexity ? 9115
=========================================
Files ? 961
Lines ? 48123
Branches ? 4677
=========================================
Hits ? 33406
Misses ? 12506
Partials ? 2211
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1044?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1044?src=pr&el=footer). Last update [9798cdb...19b00f5](https://codecov.io/gh/apache/hadoop-ozone/pull/1044?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1044: HDDS-3743. Avoid NetUtils#normalize when get DatanodeDetails from proto
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1044:
URL: https://github.com/apache/hadoop-ozone/pull/1044#discussion_r439149023
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetUtils.java
##########
@@ -59,9 +64,12 @@ public static String normalize(String path) {
+ NetConstants.PATH_SEPARATOR_STR + ": " + path);
}
+ if (len == 1 || path.charAt(len - 1) != NetConstants.PATH_SEPARATOR) {
Review comment:
Can we simplified the logic without any matcher by using apache common StringUtils?
return len == 1 ? path : StringUtils.removeEnd(path, "/");
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] runzhiwang commented on a change in pull request #1044: HDDS-3743. Avoid NetUtils#normalize when get DatanodeDetails from proto
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #1044:
URL: https://github.com/apache/hadoop-ozone/pull/1044#discussion_r439812954
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetUtils.java
##########
@@ -59,9 +64,12 @@ public static String normalize(String path) {
+ NetConstants.PATH_SEPARATOR_STR + ": " + path);
}
+ if (len == 1 || path.charAt(len - 1) != NetConstants.PATH_SEPARATOR) {
Review comment:
@xiaoyuyao Thanks for review. Updated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao merged pull request #1044: HDDS-3743. Avoid NetUtils#normalize when get DatanodeDetails from proto
Posted by GitBox <gi...@apache.org>.
xiaoyuyao merged pull request #1044:
URL: https://github.com/apache/hadoop-ozone/pull/1044
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1044: HDDS-3743. Avoid NetUtils#normalize when get DatanodeDetails from proto
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1044:
URL: https://github.com/apache/hadoop-ozone/pull/1044#discussion_r439589149
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetUtils.java
##########
@@ -59,9 +64,12 @@ public static String normalize(String path) {
+ NetConstants.PATH_SEPARATOR_STR + ": " + path);
}
+ if (len == 1 || path.charAt(len - 1) != NetConstants.PATH_SEPARATOR) {
Review comment:
You are correct. The node to location mapping is provided by script or mapping file configured and parsed by implementation of DNSToSwitchMapping. The script should usually normalize the path already. Otherwise,
the mapping can't be used consistently for the topology class to determine node locations.
E.g., multiple slash: "///" => ""
multiple slash in the middle "/a//b/c/" => "/a//b/c" which is not equal to "/a/b/c", etc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] runzhiwang commented on a change in pull request #1044: HDDS-3743. Avoid NetUtils#normalize when get DatanodeDetails from proto
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #1044:
URL: https://github.com/apache/hadoop-ozone/pull/1044#discussion_r439151595
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetUtils.java
##########
@@ -59,9 +64,12 @@ public static String normalize(String path) {
+ NetConstants.PATH_SEPARATOR_STR + ": " + path);
}
+ if (len == 1 || path.charAt(len - 1) != NetConstants.PATH_SEPARATOR) {
Review comment:
@xiaoyuyao Thanks for review. But the result of StringUtils.removeEnd("/a/b/c//", "/") is "/a/b/c/" instead of "/a/b/c". I'm not sure whether we should consider the path end with multiple "/".
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org