symat commented on a change in pull request #1251: ZOOKEEPER-3720: Fix rolling
upgrade failure (invalid protocol version)
URL: https://github.com/apache/zookeeper/pull/1251#discussion_r378303201
##########
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
##########
@@ -639,7 +657,7 @@ public void toSend(Long sid, ByteBuffer b) {
synchronized boolean connectOne(long sid, MultipleAddresses electionAddr) {
if (senderWorkerMap.get(sid) != null) {
LOG.debug("There is a connection already for server {}", sid);
- if (electionAddr.size() > 1 &&
self.isMultiAddressReachabilityCheckEnabled()) {
+ if (electionAddr.size() > 1 && self.isMultiAddressEnabled() &&
self.isMultiAddressReachabilityCheckEnabled()) {
Review comment:
you are right, actually if multi-address is disabled, then we can't have
more than one address anyway. Still, I think it is good to have the extra
condition there (if for nothing else, maybe just to make the code easier to
understand)
I will move it to the beginning.
----------------------------------------------------------------
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
