hanm commented on issue #1048: ZOOKEEPER-3188: Improve resilience to network URL: https://github.com/apache/zookeeper/pull/1048#issuecomment-526397890 nice work on rebase and doing cluster level testing on the patch a few things I'd like to discuss: * What's the impact on dynamic reconfiguration - what's allowed / working and what's not allowed / not working when reconfiguring server's address now with this feature? Once this is concluded, some unit tests and cluster level testing would be good. I think @enixon also asked about this in https://github.com/apache/zookeeper/pull/730. * Impact on upgrading / downgrading: this was discussed on the JIRA, should be tested at cluster level and document it on upgrade FAQ wiki and/or zookeeper document. * Impact on quorum TLS/kerberos feature: this was also discussed on JIRA, but not concluded yet. * Unit test coverage: I think the current unit tests don't cover well the new code path when there is multiple addresses configured. We have quorum manager test, leader election tests, and so on; but those tests are still executed with a single address configured (even though the interfaces changed to multiple address). Would be good to upgrade relevant tests and make sure we have a relevant good coverage on networking code path with multiple addresses configured. My current evaluation of the patch is the risk is fairly low if we merge this, as long as we still stick to single address configuration. Though I'd like to conclude the above discussions so we know what we will be getting into if something goes wrong.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
