You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/05/27 03:33:14 UTC
[GitHub] [helix] narendly commented on a change in pull request #1019: ConnectionTimeout change to ensure HelixManager initial connection
narendly commented on a change in pull request #1019:
URL: https://github.com/apache/helix/pull/1019#discussion_r430047949
##########
File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/ZkClient.java
##########
@@ -62,7 +62,10 @@
private static Logger LOG = LoggerFactory.getLogger(ZkClient.class);
public static final int DEFAULT_OPERATION_TIMEOUT = Integer.MAX_VALUE;
- public static final int DEFAULT_CONNECTION_TIMEOUT = 60 * 1000;
+ // Four times of DEFAULT_SESSION_TIMEOUT would give less 1/125 chance of initial connection
+ // to Zookeeper failure assuming one observer in the 5 observer Zookeeper Quorum is down
Review comment:
@kaisun2000
I also think that we should leave the constant as is and make it configurable in the system config.
Note that the HelixManager usage is _**the one you found**_. This is a public constant, and you don't know if it's being used elsewhere. So there is an issue of backward-compatibility if you change the value of the constant.
##########
File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/ZkClient.java
##########
@@ -62,7 +62,10 @@
private static Logger LOG = LoggerFactory.getLogger(ZkClient.class);
public static final int DEFAULT_OPERATION_TIMEOUT = Integer.MAX_VALUE;
- public static final int DEFAULT_CONNECTION_TIMEOUT = 60 * 1000;
+ // Four times of DEFAULT_SESSION_TIMEOUT would give less 1/125 chance of initial connection
+ // to Zookeeper failure assuming one observer in the 5 observer Zookeeper Quorum is down
+ // with one DNS name configured as connection string with 3.4.13+ Zookeeper client version
+ public static final int DEFAULT_CONNECTION_TIMEOUT = 120 * 1000;
Review comment:
+1. This is a public constant, so we need to avoid changing its value.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org