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/03/31 12:49:57 UTC
[GitHub] [hadoop-ozone] runzhiwang opened a new pull request #740:
HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
runzhiwang opened a new pull request #740: HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
URL: https://github.com/apache/hadoop-ozone/pull/740
## What changes were proposed in this pull request?
**What's the problem ?**
When create key, [getHostName()](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java#L130) by IP need reverse DNS lookup, cost about 15ms. It's also reported in [JDK-6450279](https://bugs.openjdk.java.net/browse/JDK-6450279)
**How to fix ?**
client getLocalHost().getHostName() and add it in request, server get hostname from request.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3307
## How was this patch tested?
Existed UT and IT.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on issue #740: HDDS-3307.
Improve write efficiency by avoiding reverse DNS lookup
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on issue #740: HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
URL: https://github.com/apache/hadoop-ozone/pull/740#issuecomment-607014247
> > @runzhiwang Thanks for the PR and improvement.
> > I have few comments.
> > I have one general question, now in native authorizer, the hostname is not used during checkAcls, in the future if we use, allow requests from this host like settings, then with this approach, someone can set the hostName, even though the request is not from that host. This can happen with this code change, I am not sure we will support this kind of scenario or not. Just flagging this here.
>
> @bharatviswa504 Thanks for your review. Very reasonable question. And I also doubt whether we need hostName in authorizer, because `getHostName `from IP is a block call, maybe block very long time. If we do not need hostName in authorizer, we can delete `getHostName `and also no need to `setHostName `in client.
HostName is added based on the comments from @eyang during the review of HDDS-1600. Have a look into it. From reading the current code, in NativeAuthorizer, we have not used hostName during ACL validation. MIght be some external providers like ranger might need this, not sure ranger is currently using it or not.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#740: HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #740: HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
URL: https://github.com/apache/hadoop-ozone/pull/740#discussion_r401276822
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
##########
@@ -127,7 +127,14 @@ public OMRequest getOmRequest() {
}
if (remoteAddress != null) {
- userInfo.setHostName(remoteAddress.getHostName());
+ OzoneManagerProtocolProtos.UserInfo userInfoInProtocol = getOmRequest()
Review comment:
We don't need toBuilder here, we can get UserInfo by calling getUserInfo()
Like `getOmRequest().getUserInfo()`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 edited a comment on issue #740:
HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
Posted by GitBox <gi...@apache.org>.
runzhiwang edited a comment on issue #740: HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
URL: https://github.com/apache/hadoop-ozone/pull/740#issuecomment-607011449
> @runzhiwang Thanks for the PR and improvement.
>
> I have few comments.
>
> I have one general question, now in native authorizer, the hostname is not used during checkAcls, in the future if we use, allow requests from this host like settings, then with this approach, someone can set the hostName, even though the request is not from that host. This can happen with this code change, I am not sure we will support this kind of scenario or not. Just flagging this here.
@bharatviswa504 Thanks for your review. Very reasonable question. And I also doubt whether we need hostName in authorizer, because `getHostName `from IP is a block call, maybe block very long time. If we do not need hostName in authorizer, we can delete `getHostName `and also no need to `setHostName `in client.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #740: HDDS-3307.
Improve write efficiency by avoiding reverse DNS lookup
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on issue #740: HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
URL: https://github.com/apache/hadoop-ozone/pull/740#issuecomment-607011449
> @runzhiwang Thanks for the PR and improvement.
>
> I have few comments.
>
> I have one general question, now in native authorizer, the hostname is not used during checkAcls, in the future if we use, allow requests from this host like settings, then with this approach, someone can set the hostName, even though the request is not from that host. This can happen with this code change, I am not sure we will support this kind of scenario or not. Just flagging this here.
@bharatviswa504 Thanks for your review. Very reasonable question. And I also doubt whether we need hostName in authorizer, because getHostName() from IP is a block call, maybe block a very long time, it's dangerous. If we do not need hostName in authorizer, we can delete `getHostName `and also no need to `setHostName `in client.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 edited a comment on issue #740:
HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
Posted by GitBox <gi...@apache.org>.
runzhiwang edited a comment on issue #740: HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
URL: https://github.com/apache/hadoop-ozone/pull/740#issuecomment-607011449
> @runzhiwang Thanks for the PR and improvement.
>
> I have few comments.
>
> I have one general question, now in native authorizer, the hostname is not used during checkAcls, in the future if we use, allow requests from this host like settings, then with this approach, someone can set the hostName, even though the request is not from that host. This can happen with this code change, I am not sure we will support this kind of scenario or not. Just flagging this here.
@bharatviswa504 Thanks for your review. Very reasonable question. And I also doubt whether we need hostName in authorizer, because `getHostName `from IP is a block call, maybe block a very long time. If we do not need hostName in authorizer, we can delete `getHostName `and also no need to `setHostName `in client.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#740: HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #740: HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
URL: https://github.com/apache/hadoop-ozone/pull/740#discussion_r401275915
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
##########
@@ -127,7 +127,14 @@ public OMRequest getOmRequest() {
}
if (remoteAddress != null) {
- userInfo.setHostName(remoteAddress.getHostName());
+ OzoneManagerProtocolProtos.UserInfo userInfoInProtocol = getOmRequest()
+ .toBuilder().getUserInfo();
+ if (userInfoInProtocol.getHostName().isEmpty()) {
Review comment:
If userInfoInProtocol.getHostName(), we need to set it as before right?
Like userInfo.setHostName(remoteAddress.getHostName());
As when someone uses RpcClient, this if check will not be true. So if HostName is empty from UserInfo, I think we still need to set hostname as before.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on issue #740: HDDS-3307.
Improve write efficiency by avoiding reverse DNS lookup
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on issue #740: HDDS-3307. Improve write efficiency by avoiding reverse DNS lookup
URL: https://github.com/apache/hadoop-ozone/pull/740#issuecomment-606946772
And also test failure looks related to the code change.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org