[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...

2017-05-11 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/zookeeper/pull/106


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...

2017-05-08 Thread hanm
Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/106#discussion_r115353405
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 
---
@@ -930,39 +918,36 @@ protected Election createElectionAlgorithm(int 
electionAlgorithm){
 
 //TODO: use a factory rather than a switch
 switch (electionAlgorithm) {
-case 0:
-le = new LeaderElection(this);
-break;
-case 1:
-le = new AuthFastLeaderElection(this);
-break;
-case 2:
-le = new AuthFastLeaderElection(this, true);
-break;
-case 3:
-qcm = new QuorumCnxManager(this);
-QuorumCnxManager.Listener listener = qcm.listener;
-if(listener != null){
-listener.start();
-FastLeaderElection fle = new FastLeaderElection(this, qcm);
-fle.start();
-le = fle;
-} else {
-LOG.error("Null listener when initializing cnx manager");
-}
-break;
-default:
-assert false;
+case 0:
+assert false : "Leader election algorithm type 0 is not 
supported anymore.";
--- End diff --

I like the idea of catching invalid electionAlg value when parsing config 
file. Code updated by throwing an exception when input value is bad.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...

2017-05-08 Thread hanm
Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/106#discussion_r115352994
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 
---
@@ -930,39 +918,36 @@ protected Election createElectionAlgorithm(int 
electionAlgorithm){
 
 //TODO: use a factory rather than a switch
 switch (electionAlgorithm) {
-case 0:
-le = new LeaderElection(this);
-break;
-case 1:
-le = new AuthFastLeaderElection(this);
-break;
-case 2:
-le = new AuthFastLeaderElection(this, true);
-break;
-case 3:
-qcm = new QuorumCnxManager(this);
-QuorumCnxManager.Listener listener = qcm.listener;
-if(listener != null){
-listener.start();
-FastLeaderElection fle = new FastLeaderElection(this, qcm);
-fle.start();
-le = fle;
-} else {
-LOG.error("Null listener when initializing cnx manager");
-}
-break;
-default:
-assert false;
+case 0:
+assert false : "Leader election algorithm type 0 is not 
supported anymore.";
+break;
+case 1:
+le = new AuthFastLeaderElection(this);
+break;
+case 2:
+le = new AuthFastLeaderElection(this, true);
+break;
+case 3:
+qcm = new QuorumCnxManager(this);
+QuorumCnxManager.Listener listener = qcm.listener;
+if(listener != null){
+listener.start();
+FastLeaderElection fle = new FastLeaderElection(this, 
qcm);
+fle.start();
+le = fle;
+} else {
+LOG.error("Null listener when initializing cnx 
manager");
+}
+break;
+default:
+assert false;
 }
 return le;
 }
 
 @SuppressWarnings("deprecation")
 protected Election makeLEStrategy(){
 LOG.debug("Initializing leader election protocol...");
-if (getElectionType() == 0) {
-electionAlg = new LeaderElection(this);
-}
 return electionAlg;
 }
 
--- End diff --

good catch. fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...

2017-05-08 Thread hanm
Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/106#discussion_r115353017
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 
---
@@ -809,28 +809,16 @@ synchronized public void stopLeaderElection() {
 responder.interrupt();
--- End diff --

removed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...

2017-05-08 Thread hanm
Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/106#discussion_r115353053
  
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml ---
@@ -948,15 +948,14 @@ server.3=zoo3:2888:3888
 
   (No Java system property)
 
-  Election implementation to use. A value of "0" 
corresponds
-  to the original UDP-based version, "1" corresponds to the
+  Election implementation to use. A value of "1" 
corresponds to the
   non-authenticated UDP-based version of fast leader election, 
"2"
   corresponds to the authenticated UDP-based version of fast
   leader election, and "3" corresponds to TCP-based version of
-  fast leader election. Currently, algorithm 3 is the 
default
+  fast leader election. Currently, algorithm 3 is the 
default.
   
   
-   The implementations of leader election 0, 1, and 2 
are now 
+   The implementations of leader election 1, and 2 are 
now
deprecated . We have the 
intention
   of removing them in the next release, at which point only 
the 
   FastLeaderElection will be available. 
--- End diff --

removed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...

2017-05-05 Thread arshadmohammad
Github user arshadmohammad commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/106#discussion_r115111872
  
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml ---
@@ -948,15 +948,14 @@ server.3=zoo3:2888:3888
 
   (No Java system property)
 
-  Election implementation to use. A value of "0" 
corresponds
-  to the original UDP-based version, "1" corresponds to the
+  Election implementation to use. A value of "1" 
corresponds to the
   non-authenticated UDP-based version of fast leader election, 
"2"
   corresponds to the authenticated UDP-based version of fast
   leader election, and "3" corresponds to TCP-based version of
-  fast leader election. Currently, algorithm 3 is the 
default
+  fast leader election. Currently, algorithm 3 is the 
default.
   
   
-   The implementations of leader election 0, 1, and 2 
are now 
+   The implementations of leader election 1, and 2 are 
now
deprecated . We have the 
intention
   of removing them in the next release, at which point only 
the 
   FastLeaderElection will be available. 
--- End diff --

There is at least one more reference of election alog type 0. Should be 
deleted.
Reference Text : If electionAlg is 0, then the second port is not 
  necessary


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...

2017-05-05 Thread arshadmohammad
Github user arshadmohammad commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/106#discussion_r115111914
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 
---
@@ -930,39 +918,36 @@ protected Election createElectionAlgorithm(int 
electionAlgorithm){
 
 //TODO: use a factory rather than a switch
 switch (electionAlgorithm) {
-case 0:
-le = new LeaderElection(this);
-break;
-case 1:
-le = new AuthFastLeaderElection(this);
-break;
-case 2:
-le = new AuthFastLeaderElection(this, true);
-break;
-case 3:
-qcm = new QuorumCnxManager(this);
-QuorumCnxManager.Listener listener = qcm.listener;
-if(listener != null){
-listener.start();
-FastLeaderElection fle = new FastLeaderElection(this, qcm);
-fle.start();
-le = fle;
-} else {
-LOG.error("Null listener when initializing cnx manager");
-}
-break;
-default:
-assert false;
+case 0:
+assert false : "Leader election algorithm type 0 is not 
supported anymore.";
+break;
+case 1:
+le = new AuthFastLeaderElection(this);
+break;
+case 2:
+le = new AuthFastLeaderElection(this, true);
+break;
+case 3:
+qcm = new QuorumCnxManager(this);
+QuorumCnxManager.Listener listener = qcm.listener;
+if(listener != null){
+listener.start();
+FastLeaderElection fle = new FastLeaderElection(this, 
qcm);
+fle.start();
+le = fle;
+} else {
+LOG.error("Null listener when initializing cnx 
manager");
+}
+break;
+default:
+assert false;
 }
 return le;
 }
 
 @SuppressWarnings("deprecation")
 protected Election makeLEStrategy(){
 LOG.debug("Initializing leader election protocol...");
-if (getElectionType() == 0) {
-electionAlg = new LeaderElection(this);
-}
 return electionAlg;
 }
 
--- End diff --


org.apache.zookeeper.server.quorum.QuorumPeerConfig.parseDynamicConfig(Properties,
 int, boolean, boolean)
has a logic related to election algo type 0. This should be removed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...

2017-05-05 Thread arshadmohammad
Github user arshadmohammad commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/106#discussion_r115111921
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 
---
@@ -809,28 +809,16 @@ synchronized public void stopLeaderElection() {
 responder.interrupt();
--- End diff --

The import java.net.SocketException is never used


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...

2017-05-05 Thread arshadmohammad
Github user arshadmohammad commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/106#discussion_r115112178
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 
---
@@ -930,39 +918,36 @@ protected Election createElectionAlgorithm(int 
electionAlgorithm){
 
 //TODO: use a factory rather than a switch
 switch (electionAlgorithm) {
-case 0:
-le = new LeaderElection(this);
-break;
-case 1:
-le = new AuthFastLeaderElection(this);
-break;
-case 2:
-le = new AuthFastLeaderElection(this, true);
-break;
-case 3:
-qcm = new QuorumCnxManager(this);
-QuorumCnxManager.Listener listener = qcm.listener;
-if(listener != null){
-listener.start();
-FastLeaderElection fle = new FastLeaderElection(this, qcm);
-fle.start();
-le = fle;
-} else {
-LOG.error("Null listener when initializing cnx manager");
-}
-break;
-default:
-assert false;
+case 0:
+assert false : "Leader election algorithm type 0 is not 
supported anymore.";
--- End diff --

Another option is  validate electionAlg in 
org.apache.zookeeper.server.quorum.QuorumPeerConfig and exit from there for 
wrong input and remove the case from here 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...

2017-05-05 Thread arshadmohammad
Github user arshadmohammad commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/106#discussion_r115112169
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 
---
@@ -930,39 +918,36 @@ protected Election createElectionAlgorithm(int 
electionAlgorithm){
 
 //TODO: use a factory rather than a switch
 switch (electionAlgorithm) {
-case 0:
-le = new LeaderElection(this);
-break;
-case 1:
-le = new AuthFastLeaderElection(this);
-break;
-case 2:
-le = new AuthFastLeaderElection(this, true);
-break;
-case 3:
-qcm = new QuorumCnxManager(this);
-QuorumCnxManager.Listener listener = qcm.listener;
-if(listener != null){
-listener.start();
-FastLeaderElection fle = new FastLeaderElection(this, qcm);
-fle.start();
-le = fle;
-} else {
-LOG.error("Null listener when initializing cnx manager");
-}
-break;
-default:
-assert false;
+case 0:
+assert false : "Leader election algorithm type 0 is not 
supported anymore.";
--- End diff --

I think we should log error here and make sure the server jvm exit in this 
case


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...

2016-11-15 Thread hanm
GitHub user hanm opened a pull request:

https://github.com/apache/zookeeper/pull/106

ZOOKEEPER-1932: Remove deprecated LeaderElection class.

The motivation of removing LeaderElection class:
* It has been long deprecated and no one uses it.
* Tests around it is flaky.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hanm/zookeeper ZOOKEEPER-1932

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/106.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #106


commit f6f1ad497c71b54ae8944e7d1b1726e66b8153f6
Author: Michael Han 
Date:   2016-11-15T18:57:04Z

ZOOKEEPER-1932: Remove deprecated LeaderElection class.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---