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 ra
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:
https://lists.apache.org/thread.html/8d3dcd84b0ad47563e2cf2
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 updat
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 log
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 a
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 InetSocketA
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 o
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 i
25 matches
Mail list logo