Dynamic UDP target host address

[m4ce]

This PR tries to solve #480 by refreshing the target UDP host address every 5m. I am not sure if this is the best approach but that's all I could come up with!

[codecov-io]

Codecov Report

Merging #481 into master will decrease coverage by 0.02%.
The diff coverage is 83.33%.

@@             Coverage Diff              @@
##             master     #481      +/-   ##
============================================
- Coverage     87.29%   87.27%   -0.03%     
  Complexity      357      357              
============================================
  Files            24       24              
  Lines          1496     1501       +5     
  Branches        164      165       +1     
============================================
+ Hits           1306     1310       +4     
  Misses          124      124              
- Partials         66       67       +1

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/org/influxdb/InfluxDBFactory.java	100% <ø> (ø)	6 <0> (ø)	⬇️
src/main/java/org/influxdb/impl/InfluxDBImpl.java	89.45% <83.33%> (-0.13%)	73 <1> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dab365a...cb46702. Read the comment docs.

[lxhoan]

Hi @fmachado , I think resolving and caching IP address for datagram is unnecessary (and obviously falls defective this case)
In my opinion, this snippet

datagramSocket.send(new DatagramPacket(bytes, bytes.length, hostAddress, udpPort));

hostAddress should be changed to SocketAdress, something like this

datagramSocket.send(new DatagramPacket(bytes, bytes.length, new InetSocketAddress(hostname,  udpPort)));

What do you think ?

[m4ce]

@lxhoan, that works well only if one has batching enabled. If batching is disabled, it will resolve the hostname every time an UDP write is performed. It can result in denial of service to the DNS service if one sends out a lot of metrics in a short interval of time.

[lxhoan]

@m4ce, there some other dns caching levels down to the IP layer (at least at JVM and your OS), So I think we can be safe
(for java you can refer to this https://docs.oracle.com/javase/6/docs/technotes/guides/net/properties.html)

Otherwise we must be meticulous when introducing more configuration to influxdb (I mean this one ADDRESS_LOOKUP_INTERVAL)

[lxhoan]

Hi @fmachado , if you can arrange time please take a look on it
I think this is a realistic use case that influxdb-java should consider to support

[fmachado]

@m4ce as far as I could understand from your issue ticket and this PR, you can solve your problem by adding a parameter -Dnetworkaddress.cache.ttl=300 to the JVM (as mentioned by @lxhoan and here).

You can also apply this configuration by calling Security.setProperty (javadoc) but I don't know what happens with already resolved names.

For the reasons mentioned before, I vote for do not merge this PR.

[fmachado]

I'm closing this issue but feel free to reopen it if you think your issue cannot be fixed with the way we mentioned here.

[m4ce]

@fmachado , sorry to comment on a close issue. I think you suggestion would only work if we changed the code to be:

datagramSocket.send(new DatagramPacket(bytes, bytes.length, new InetSocketAddress(hostname, udpPort)));

Just like @lxhoan suggested above.

[m4ce]

I see this change was implemented on Sept 5th. We are good then!