jiajunwang commented on a change in pull request #1328:
URL: https://github.com/apache/helix/pull/1328#discussion_r487218755
##########
File path:
helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/ZkHelixClusterVerifier.java
##########
@@ -335,6 +352,11 @@ public B setZkAddr(String zkAddress) {
return setZkAddress(zkAddress);
}
+ public B setWaitTillVerify(int waitPeriod) {
+ _coolDown = waitPeriod;
Review comment:
Let's just call it "waitPeriodTillVerify. Cooldown is not an obvious
name.
##########
File path:
helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/BestPossibleExternalViewVerifier.java
##########
@@ -224,12 +224,15 @@ public boolean verifyByZkCallback(long timeout) {
@Override
protected synchronized boolean verifyState() {
try {
+ LOG.debug("Verifier start verifyState at {}",
System.currentTimeMillis());
Review comment:
As we discussed, please remove the debug logs if not absolutely
necessary for long term usage.
##########
File path:
helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/ZkHelixClusterVerifier.java
##########
@@ -187,6 +190,14 @@ public boolean verifyByZkCallback() {
* @return
*/
public boolean verifyByPolling(long timeout, long period) {
Review comment:
I believe you can and should add the same wait for verifyByZkCallback
too.
The logic of verifyByZkCallback is that
1. subscribe to changes.
2. no matter if change happens, verify the state for once.
3. if subscription results in some notification, then verify in parallel.
4. any of the paths succeed, then the verify succeeds.
In this case, we shall wait until the first subscribe to ensure the
waitTillVerify config takes effect. This won't cause any problem with verifying
logic. Since if the change happens before we subscribe, we are still fine
because the first default verify check will be done once at least.
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]