You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by an...@apache.org on 2019/07/02 05:34:39 UTC
[zookeeper] branch master updated: ZOOKEEPER-3240: Close socket on Learner shutdown to avoid dangling so…
This is an automated email from the ASF dual-hosted git repository.
andor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 96eefaf ZOOKEEPER-3240: Close socket on Learner shutdown to avoid dangling so…
96eefaf is described below
commit 96eefafce7ed036d3ab4a7a6d43792ce433bbfad
Author: Brian Nixon <ni...@fb.com>
AuthorDate: Tue Jul 2 07:34:21 2019 +0200
ZOOKEEPER-3240: Close socket on Learner shutdown to avoid dangling so…
…cket
Author: Brian Nixon <ni...@fb.com>
Reviewers: hanm@apache.org, eolivelli@apache.org, andor@apache.org
Closes #996 from enixon/learner-close-socket and squashes the following commits:
57f5e891 [Brian Nixon] delete ReconfigFailureCasesTest::testLeaderTimesoutOnNewQuorum as it relies on the fixed buggy behavior
1598b3ad [Brian Nixon] remove extraneous check on socket status
381889da [Brian Nixon] ZOOKEEPER-3240: Close socket on Learner shutdown to avoid dangling socket
---
.../apache/zookeeper/server/quorum/Follower.java | 6 +--
.../apache/zookeeper/server/quorum/Learner.java | 11 ++++++
.../apache/zookeeper/server/quorum/Observer.java | 6 +--
.../server/quorum/QuorumPeerMainTest.java | 3 ++
.../server/quorum/ReconfigFailureCasesTest.java | 43 ----------------------
5 files changed, 16 insertions(+), 53 deletions(-)
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Follower.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Follower.java
index 16d0384..f3a8ffe 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Follower.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Follower.java
@@ -116,11 +116,7 @@ public class Follower extends Learner{
}
} catch (Exception e) {
LOG.warn("Exception when following the leader", e);
- try {
- sock.close();
- } catch (IOException e1) {
- e1.printStackTrace();
- }
+ closeSocket();
// clear pending revalidations
pendingRevalidations.clear();
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
index 64ff0fb..51979aa 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
@@ -673,6 +673,7 @@ public class Learner {
self.setZooKeeperServer(null);
self.closeAllConnections();
self.adminServer.setZooKeeperServer(null);
+ closeSocket();
// shutdown previous zookeeper
if (zk != null) {
zk.shutdown();
@@ -682,4 +683,14 @@ public class Learner {
boolean isRunning() {
return self.isRunning() && zk.isRunning();
}
+
+ void closeSocket() {
+ try {
+ if (sock != null) {
+ sock.close();
+ }
+ } catch (IOException e) {
+ LOG.warn("Ignoring error closing connection to leader", e);
+ }
+ }
}
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Observer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Observer.java
index 6e84128..907aba8 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Observer.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Observer.java
@@ -122,11 +122,7 @@ public class Observer extends Learner{
}
} catch (Exception e) {
LOG.warn("Exception when observing the leader", e);
- try {
- sock.close();
- } catch (IOException e1) {
- e1.printStackTrace();
- }
+ closeSocket();
// clear pending revalidations
pendingRevalidations.clear();
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java
index 146773b..9c4d276 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java
@@ -827,7 +827,10 @@ public class QuorumPeerMainTest extends QuorumPeerTestBase {
int leader = servers.findLeader();
Map<Long, Proposal> outstanding = servers.mt[leader].main.quorumPeer.leader.outstandingProposals;
// increase the tick time to delay the leader going to looking
+ int previousTick = servers.mt[leader].main.quorumPeer.tickTime;
servers.mt[leader].main.quorumPeer.tickTime = LEADER_TIMEOUT_MS;
+ // let the previous tick on the leader exhaust itself so the new tick time takes effect
+ Thread.sleep(previousTick);
LOG.warn("LEADER {}", leader);
for (int i = 0; i < SERVER_COUNT; i++) {
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigFailureCasesTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigFailureCasesTest.java
index a0b9760..bd9e588 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigFailureCasesTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigFailureCasesTest.java
@@ -160,49 +160,6 @@ public class ReconfigFailureCasesTest extends QuorumPeerTestBase {
}
/*
- * Tests that if a quorum of a new config is synced with the leader and a reconfig
- * is allowed to start but then the new quorum is lost, the leader will time out and
- * we go to leader election.
- */
- @Test
- public void testLeaderTimesoutOnNewQuorum() throws Exception {
- qu = new QuorumUtil(1); // create 3 servers
- qu.disableJMXTest = true;
- qu.startAll();
- ZooKeeper[] zkArr = ReconfigTest.createHandles(qu);
- ZooKeeperAdmin[] zkAdminArr = ReconfigTest.createAdminHandles(qu);
-
- List<String> leavingServers = new ArrayList<String>();
- leavingServers.add("3");
- qu.shutdown(2);
- try {
- // Since we just shut down server 2, its still considered "synced"
- // by the leader, which allows us to start the reconfig
- // (PrepRequestProcessor checks that a quorum of the new
- // config is synced before starting a reconfig).
- // We try to remove server 3, which requires a quorum of {1,2,3}
- // (we have that) and of {1,2}, but 2 is down so we won't get a
- // quorum of new config ACKs.
- zkAdminArr[1].reconfigure(null, leavingServers, null, -1, null);
- Assert.fail("Reconfig should have failed since we don't have quorum of new config");
- } catch (KeeperException.ConnectionLossException e) {
- // We expect leader to lose quorum of proposed config and time out
- } catch (Exception e) {
- Assert.fail("Should have been ConnectionLossException!");
- }
-
- // The leader should time out and remaining servers should go into
- // LOOKING state. A new leader won't be established since that
- // would require completing the reconfig, which is not possible while
- // 2 is down.
- Assert.assertEquals(QuorumStats.Provider.LOOKING_STATE,
- qu.getPeer(1).peer.getServerState());
- Assert.assertEquals(QuorumStats.Provider.LOOKING_STATE,
- qu.getPeer(3).peer.getServerState());
- ReconfigTest.closeAllHandles(zkArr, zkAdminArr);
- }
-
- /*
* Converting an observer into a participant may sometimes fail with a
* NewConfigNoQuorum exception. This test-case demonstrates the scenario.
* Current configuration is (A, B, C, D), where A, B and C are participant