You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by ijokarumawak <gi...@git.apache.org> on 2016/12/20 03:04:15 UTC
[GitHub] nifi pull request #1342: NIFI-2585 Add attributes to track s2s host and port
GitHub user ijokarumawak opened a pull request:
https://github.com/apache/nifi/pull/1342
NIFI-2585 Add attributes to track s2s host and port
I left the four commits to show the history of how this evolved. We can squash Bryan's two commits when merging, but we wanted Randy to get credit for starting the work on this ticket.
While reviewing #1320, I found few concerns as shown in the table below.
Let's say there are `client.nifi` and `server.nifi` transferring files each other using S2S. This table shows how `s2s.address` attribute values are set in each case with #1320:
| Transfer Protocol | Pulled from a remote OutputPort to S2S client | Received via an InputPort at S2S server |
|-------------------|-----------------------------------------------|--------------------------------------------|
| RAW | Server's hostname and port are the one that the client gets when it received remote NiFi peers. Those values are defined in server's nifi.properties. <br/> `nifi.remote.input.host:nifi.remote.input.socket.port` <br/> e.x. `server.nifi:8081` | Client's hostname and port is used. But due to the existing logic which parses URL string and extract hostname and port, if the URL string contains illegal character such as underscore, the result becomes null, then converted with `unknown`. This happens with Docker as container hostname often contain underscore. <br/> e.x. `client.nifi:58034`, `unknown:unknown` (if hostname contains underscore) <br/> **FIX 1** <br/> `client_nifi:58034` (even if hostname contains underscore)|
| HTTP | Same as RAW protocol, but uses `nifi.web.http(s).port`. <br/> e.x. `server.nifi:8080`| Client hostname and port are retrieved from HTTP request object. However, it returns IP address string representation instead of hostname. Probably performance reason. <br/> e.x. `192.168.0.33:59946` <br/> **FIX 2** <br/> `client.nifi:59946`.|
## FIX 1
As reporte in [the previous PR comment](https://github.com/apache/nifi/pull/1307#issuecomment-266442657) I got 'unknown' hostname and port with Docker containers. While this can be handled as a corner case since it's not allowed by the URL specification, I think it'd be better if we can be lenient here to support Docker environment. I confirmed provenance event also failed to resolve hostname and produced provenance details with null hostname.
This can be improved by changing Peer's constructor. Currently, it parses peerUrl then use its hostname and port, but the same information should be retrieved from PeerDescriptor without parsing the URL. Also, SocketRemoteSiteListener uses server's hostname and port for PeerDescription, but it seems it's not correct, those should be client's.
This commit modifies SocketRemoteSiteListener to use PeerDescriptor instead of parsing URL string.
## FIX 2
This commit modifies DataTransferResource to resolve hostname from IP address using InetAddress when HTTP transport protocol is used, as RAW resolves hostname from socket, using InetAddress.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ijokarumawak/nifi nifi-2585
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/1342.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1342
----
commit c6f0621f985b6deddadc616307f192c1ddf1060d
Author: Randy Gelhausen <rg...@gmail.com>
Date: 2016-12-07T07:18:09Z
NIFI-2585: Add attributes to track where a flow file came from when receiving over site-to-site
commit 841c4c80e42f09e7dc96552c661dde5a81ab7c08
Author: Bryan Bende <bb...@apache.org>
Date: 2016-12-08T20:17:06Z
NIFI-2585 Moving attributes into loop in AbstractFlowFileServerProtocol, and also updating StandardRemoteGroupPort to apply the same attributes when doing a pull-based site-to-site.
commit 0e3a29984d1abf94ba86d98fbe283e9671de1671
Author: Bryan Bende <bb...@apache.org>
Date: 2016-12-12T14:19:22Z
NIFI-2585 Adding checks in case host and port are not known
commit c206f266c38e537c9434d7ff66ad5e19ceb01e1c
Author: Koji Kawamura <ij...@apache.org>
Date: 2016-12-20T02:19:22Z
NIFI-2585: Add attributes to track s2s host and port
- Removed host and port field from Peer since the same information is
available in PeerDescription
- Refactored variable names in SocketRemoteSiteListener to improve readability
- Changed how SocketRemoteSiteListener constructs PeerDescription
instance. It used to use hard-coded 'localhost' as hostname, and
getPort() which returns server's port. Since the peer is a remote peer,
i.e the client, it should be client hostname and port.
- Added hostname resolution at DataTransferResource to make s2s.host
value consistent with RAW transport. Without this, RAW uses hostname
while HTTP uses IP address. It will be hard to be used from downstream flows.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #1342: NIFI-2585 Add attributes to track s2s host and port
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/1342
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1342: NIFI-2585 Add attributes to track s2s host and port
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/1342
@ijokarumawak yes they were both running on my laptop which is why I was wondering how the pull model ends up with the hostname, and the push model ends up with the ip address. I'll keep digging.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1342: NIFI-2585 Add attributes to track s2s host and port
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/1342
Thanks @ijokarumawak !
I was testing this and I noticed that whenever I do a push model from nifi-1 to nifi-2, the value of s2s.hostname on the flow file in nifi-2 ends up being the IP address of my laptop.
Whenever I do a pull model, where nifi-2 pulls from nifi-1, the value of s2s.hostname on the flow file in nifi-2 ends up being the hostname of my laptop.
I don't know if this is important or not, but I know you made some code changes to try and resolve the hostname, so would you expect this behavior?
In my setup I haven't changed much configuration... I set the remote port in nifi.properties of nifi-1, and the same thing in nifi-2, and also change web port in nifi-2, everything else is using defaults.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1342: NIFI-2585 Add attributes to track s2s host and port
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/1342
@bbende Thanks for the comment. I reconsidered about s2s.address property, and by focusing only on the downstream flow, I think it probably doesn't matter whether the flow file came into the flow either by pulling or receiving it.
If one needs to distinguish the direction in downstream flow, they can do so by adding UpdateAttribute to add whatever context they need. I think it's better to keep the number of attributes that NiFi framework create automatically at minimum. Removing feature from framework may be harder than adding it.
So, I will keep the attribute names as they are now. Will update the PR with additional unit tests later. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1342: NIFI-2585 Add attributes to track s2s host and port
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/1342
I used InetAddress.getHostName() to convert ip address to hostname, but if it can't resolve hostname, it just returns ip address. It depends on network environment. If `$ host <ipaddress>` command can't resolve the hostname, then InetAddress.getHostName wouldn't be able to resolve, too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1342: NIFI-2585 Add attributes to track s2s host and port
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/1342
@bbende @randerzander
Sorry for taking long, it took time to confirm results and consider how the result should be..
Please take a look on the table in previous comment, and let me know how you think. I added additional commit to address those concerns.
In addition to that, I'd like to have attribute key names like `s2s.remote.address` or `s2s.client.address` and `s2s.server.address` to describe which host the value represents more clearly.
Also, I'd like to add some unit test cases to protect it from breaking in the future. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1342: NIFI-2585 Add attributes to track s2s host and port
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/1342
I think the issue was that I had left nifi.remote.input.host blank on both instances. So in the push model it was getting the ip address of the incoming connection which was my laptop's internal IP that doesn't resolve to a hostname, and in the pull model it was receiving the hostname of the peer from the peer itself (line 228 of SocketClientProtocol) which happened to be hostname of my laptop.
When I set nifi.remote.input.host to localhost for both of the instances, then I always get localhost in the attributes which is good.
I'm a +1, will go ahead and merge. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1342: NIFI-2585 Add attributes to track s2s host and port
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/1342
@ijokarumawak thanks for digging in to this! I closed the other PR. Everything you described makes sense...
I'm on the fence about the attribute names... lets say we have MiNiFi -> s2s -> NiFi, the s2s.address is always going to be the address of MiNiFi, but from the address we don't know if MiNiFi was the client (push to NiFi) vs the server (NiFi pull from MiNiFi). I was originally thinking of having a third attribute like s2s.direction which could have values of "push" or "pull" which then gives you context about the host and port. What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1342: NIFI-2585 Add attributes to track s2s host and port
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/1342
Hi @bbende
I've updated unit tests. Please review the tests and merge it if it looks good. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1342: NIFI-2585 Add attributes to track s2s host and port
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/1342
@bbende Are those nifi-1 and nifi-2 running on your laptop with different port configurations? Then I'd expect s2s.hostname will be the laptop hostname. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---