[GitHub] [zookeeper] hanm commented on a change in pull request #1444: ZOOKEEPER-3922: The introduction of the oracle, a failure detector.

[GitBox <gi...@apache.org>]

hanm commented on a change in pull request #1444:
URL: https://github.com/apache/zookeeper/pull/1444#discussion_r508150587



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java
##########
@@ -39,6 +44,17 @@
     Map<Long, QuorumServer> getVotingMembers();
     Map<Long, QuorumServer> getObservingMembers();
     boolean equals(Object o);
+    /*
+    * Only QuorumOracleMaj will implement these methods. Other class will raise warning if the methods are called and
+    * return false always.
+    * */
+    boolean updateNeedOracle(List<LearnerHandler> forwardingFollowers);

Review comment:
       We can use default methods for these newly added methods so we don't have to copy paste the dummy implementation for 2 out of 3 implementations.

##########
File path: zookeeper-docs/src/main/resources/markdown/zookeeperOracleQuorums.md
##########
@@ -0,0 +1,128 @@
+<!--
+Copyright 2002-2004 The Apache Software Foundation
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+//-->
+
+# Introduction to Oracle Quorum
+
+The introduction to Oracle Quorum increases the availability of a cluster of 2 ZooKeeper instances with a failure detector
+ as known as the Oracle. The Oracle is designed to grant the permission to the instance which is the only remaining instance
+in a 2-node configuration when the other instance is identified as faulty by the fail detector, the Oracle.
+
+## The implementation of the Oracle

Review comment:
       Do we have a canonical reference implementation of such an Oracle somewhere? After reading the doc, I only know an Oracle is a failure detector but I am clueless on how to implement one for myself, or where to get one if I want to try this feature. 

##########
File path: zookeeper-server/src/test/java/org/apache/zookeeper/test/QuorumBase.java
##########
@@ -71,13 +79,14 @@
     protected boolean localSessionsEnabled = false;
     protected boolean localSessionsUpgradingEnabled = false;
 
+
     @BeforeEach
     @Override
     public void setUp() throws Exception {
-        setUp(false);
+        setUp(false, true);
     }
 
-    protected void setUp(boolean withObservers) throws Exception {
+    protected void setUp(boolean withObservers, boolean withOracle) throws Exception {

Review comment:
       I suggest we refactor this with JUNIT parameterized tests. We have quite a few examples in existing unit tests on how to use it.




----------------------------------------------------------------
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