[GitHub] zookeeper pull request #106: ZOOKEEPER-1932: Remove deprecated LeaderElectio...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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. ---