Github user ijuma commented on the issue:
https://github.com/apache/zookeeper/pull/451
Really happy to see this in a released version, thanks everyone.
@anmolnar after testing this with Apache Kafka, it seems like the
combination of a reasonably long backoff (1 second) with
Github user ijuma commented on the issue:
https://github.com/apache/zookeeper/pull/451
Also, as I'm sure many are wondering, a discussion about the next release
including this fix is taking place in the mailing list:
Github user ijuma commented on the issue:
https://github.com/apache/zookeeper/pull/451
Thanks @anmolnar, @fpj and @hanm for getting this in.
---
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/451
Thanks @hanm
---
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/451
Committed to branch-3.4. Please close the pull request @anmolnar.
Please port it to branch-3.5 and master (if you like :-), and create
necessary JIRAs for auxiliary tasks (documentation
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/451
@hanm I updated the pull request according to your suggestions. Please take
a look.
I also updated javadoc accordingly. ZooKeeper docs could be updated in a
separate PR as suggested.
---
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/451
The patch looks good, I think we just need to get the public API change
issue settled before we can merge this in.
---
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/451
There are also some initial comments about JVM DNS caching
>> Re-resolving at StaticHostProvider level may not be sufficient as
InetAddress.getAllByName(String host) itself uses a Java-level
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/451
I am reviewing this and will merge before Memorial Day if no other issues.
---
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/451
I agree with @ijuma
@afine You were involved in this patch too. Are you willing to merge it in
@fpj 's absence?
---
Github user ijuma commented on the issue:
https://github.com/apache/zookeeper/pull/451
If there are no other concerns, it would be great for this to be merged.
---
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/451
@fpj I think this patch is ready for merging as it is. Are you still having
concerns?
---
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/451
@afine @ijuma I've finished refactoring of StaticHostProvider.
The implementation follows that I explained in my email as **Option-3**:
> - Do not cache IPs,
> - Shuffle the
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/451
@ijuma I feel your pain. :)
No worries, I'm on it. Doing my best to push committers and others to
review changes.
Additionally I'd like to make a small refactoring to the proposed
Github user ijuma commented on the issue:
https://github.com/apache/zookeeper/pull/451
@anmolnar, it's been more than 1 month since the last comment on this PR.
Is there anything that still needs to be addressed? The original PR was
submitted in January 2017 and it got stalled after
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/451
@ijuma Sure, no problem.
I'm waiting for some feedback from the community here and on the mailing
list and I hope I can commit soon.
---
Github user ijuma commented on the issue:
https://github.com/apache/zookeeper/pull/451
Thanks for picking this up @anmolnar, looking forward to this being fixed.
:)
---
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/451
Going one step back I wonder why we try to deal with multiple addresses at
all.
HostProvider should just make a transformation from unresolved
InetSocketAddresses to resolved
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/451
@jeffwidman
> Should this PR be targeting branch-3.4 or target trunk and then backport
to the 3.4 series?
The original PR targets 3.4 which is explained in a comment from @fpj
Github user jeffwidman commented on the issue:
https://github.com/apache/zookeeper/pull/451
Should this PR be targeting `branch-3.4` or target `trunk` and then
backport to the 3.4 series?
---
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/451
@phunt @afine Did you have a chance to take a look?
I think we've addressed all issues that were mentioned in the original PR.
---
Github user mfenes commented on the issue:
https://github.com/apache/zookeeper/pull/451
Looking at the static initialization block in InetAddressCachePolicy more
deeply, the default TTL is 30 seconds if there is no SecurityManager installed.
So caching a positive lookup forever in
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/451
Just confirmed on 3.4 branch: ZK uses 30 secs cache TTL on my mac.
---
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/451
@mfenes The only solution I can think of is to set DNS cache TTL
`networkaddress.cache.ttl` to a configurable, non-infinite value.
---
Github user mfenes commented on the issue:
https://github.com/apache/zookeeper/pull/451
Re-resolving at StaticHostProvider level may not be sufficient as
InetAddress.getAllByName(String host) itself uses a Java-level cache inside
InetAddress and turns to name service (e.g. DNS) only
25 matches
Mail list logo
