ZOOKEEPER-698. intermittent JMX test failures due to not verifying QuorumPeer shutdown

git-svn-id: https://svn.apache.org/repos/asf/hadoop/zookeeper/trunk@922459 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -276,6 +276,9 @@ BUGFIXES:
   ZOOKEEPER-121. SyncRequestProcessor is not closing log stream during
   shutdown (mahadev)
 
+  ZOOKEEPER-698. intermittent JMX test failures due to not verifying QuorumPeer
+  shutdown (phunt)
+
 IMPROVEMENTS:
   ZOOKEEPER-473. cleanup junit tests to eliminate false positives due to
   "socket reuse" and failure to close client (phunt via mahadev)

src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java

@@ -20,9 +20,9 @@
 package org.apache.zookeeper.server.quorum;
 
 import java.nio.ByteBuffer;
-import java.util.Map;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
 

src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java

@@ -63,7 +63,7 @@ public class QuorumPeerMain {
 
     private static final String USAGE = "Usage: QuorumPeerMain configfile";
 
-    private QuorumPeer quorumPeer;
+    protected QuorumPeer quorumPeer;
 
     /**
      * To start the replicated server specify the configuration file name on
@@ -147,8 +147,4 @@ public class QuorumPeerMain {
           LOG.warn("Quorum Peer interrupted", e);
       }
     }
-
-    protected void shutdown() {
-        quorumPeer.shutdown();
-    }
 }

src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java

@@ -21,12 +21,6 @@ package org.apache.zookeeper.server.quorum;
 import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT;
 
 import java.io.ByteArrayOutputStream;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
-import java.io.FileWriter;
-import java.io.IOException;
-import java.io.InputStreamReader;
 import java.io.LineNumberReader;
 import java.io.StringReader;
 import java.net.InetSocketAddress;
@@ -34,20 +28,15 @@ import java.nio.ByteBuffer;
 import java.nio.channels.SocketChannel;
 import java.util.regex.Pattern;
 
-import junit.framework.TestCase;
-
 import org.apache.log4j.Layout;
 import org.apache.log4j.Level;
 import org.apache.log4j.Logger;
 import org.apache.log4j.WriterAppender;
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.PortAssignment;
-import org.apache.zookeeper.WatchedEvent;
-import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.ZooDefs.Ids;
 import org.apache.zookeeper.test.ClientBase;
-import org.apache.zookeeper.test.ClientTest;
 import org.junit.Test;
 
 
@@ -56,13 +45,6 @@ import org.junit.Test;
  *
  */
 public class QuorumPeerMainTest extends QuorumPeerTestBase {
-   
-    public static  class TestQPMain extends QuorumPeerMain {
-        public void shutdown() {
-            super.shutdown();
-        }
-    }
-
     /**
      * Verify the ability to start a cluster.
      */

src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java

@@ -25,13 +25,13 @@ import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
 
+import junit.framework.TestCase;
+
 import org.apache.log4j.Logger;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.server.quorum.QuorumPeerMainTest.TestQPMain;
 import org.apache.zookeeper.test.ClientBase;
-
-import junit.framework.TestCase;
+import org.apache.zookeeper.test.QuorumBase;
 
 /**
  * Has some common functionality for tests that work with QuorumPeers.
@@ -44,7 +44,14 @@ public class QuorumPeerTestBase extends TestCase implements Watcher {
     public void process(WatchedEvent event) {
         // ignore for this test
     }
-    
+
+    public static  class TestQPMain extends QuorumPeerMain {
+        public void shutdown() {
+            // ensure it closes - in particular wait for thread to exit
+            QuorumBase.shutdown(quorumPeer);
+        }
+    }
+
     public static class MainThread extends Thread {
         final File confFile;
         final TestQPMain main;

src/java/test/org/apache/zookeeper/test/FLENewEpochTest.java

@@ -134,8 +134,7 @@ public class FLENewEpochTest extends TestCase {
                         break;
                     case 2:
                         LOG.info("Third peer, shutting it down");
-                        ((FastLeaderElection) peer.getElectionAlg()).shutdown();
-                        peer.shutdown();
+                        QuorumBase.shutdown(peer);
                         flag = false;
                         round[2] = 1;
                         finish3.release();

src/java/test/org/apache/zookeeper/test/FLERestartTest.java

@@ -133,7 +133,7 @@ public class FLERestartTest extends TestCase {
                     case 0:
                         if(peerRound == 0){
                             LOG.info("First peer, shutting it down");
-                            peer.shutdown();
+                            QuorumBase.shutdown(peer);
                             ((FastLeaderElection) restartThreads.get(i).peer.getElectionAlg()).shutdown();
 
                             peer = new QuorumPeer(peers, tmpdir[i], tmpdir[i], port[i], 3, i, 2, 2, 2);

src/java/test/org/apache/zookeeper/test/FLETest.java

@@ -92,8 +92,7 @@ public class FLETest extends TestCase {
     public void tearDown() throws Exception {
         for (int i = 0; i < threads.size(); i++) {
             leThread = threads.get(i);
-            ((FastLeaderElection) leThread.peer.getElectionAlg()).shutdown();
-            leThread.peer.shutdown();
+            QuorumBase.shutdown(leThread.peer);
         }
         LOG.info("FINISHED " + getName());
     }

src/java/test/org/apache/zookeeper/test/FLEZeroWeightTest.java

@@ -91,11 +91,10 @@ public class FLEZeroWeightTest extends TestCase {
     protected void tearDown() throws Exception {
         for(int i = 0; i < threads.size(); i++) {
             LEThread leThread = threads.get(i);
-            ((FastLeaderElection) leThread.peer.getElectionAlg()).shutdown();
             // shutdown() has to be explicitly called for every thread to
             // make sure that resources are freed properly and all fixed network ports
             // are available for other test cases
-            leThread.peer.shutdown();
+            QuorumBase.shutdown(leThread.peer);
         }
         LOG.info("FINISHED " + getName());
     }

src/java/test/org/apache/zookeeper/test/HierarchicalQuorumTest.java

@@ -29,7 +29,6 @@ import java.util.Set;
 import org.apache.log4j.Logger;
 import org.apache.zookeeper.PortAssignment;
 import org.apache.zookeeper.TestableZooKeeper;
-import org.apache.zookeeper.server.quorum.FastLeaderElection;
 import org.apache.zookeeper.server.quorum.QuorumPeer;
 import org.apache.zookeeper.server.quorum.QuorumPeer.QuorumServer;
 import org.apache.zookeeper.server.quorum.flexible.QuorumHierarchical;
@@ -271,18 +270,7 @@ public class HierarchicalQuorumTest extends ClientBase {
     }
 
     protected void shutdown(QuorumPeer qp) {
-        try {
-            ((FastLeaderElection) qp.getElectionAlg()).shutdown();
-            LOG.info("Done with leader election");
-            qp.shutdown();
-            LOG.info("Done with quorum peer");
-            qp.join(30000);
-            if (qp.isAlive()) {
-                fail("QP failed to shutdown in 30 seconds");
-            }
-        } catch (InterruptedException e) {
-            LOG.debug("QP interrupted", e);
-        }
+        QuorumBase.shutdown(qp);
     }
 
     protected TestableZooKeeper createClient()

src/java/test/org/apache/zookeeper/test/LENonTerminateTest.java

@@ -35,14 +35,11 @@ import junit.framework.TestCase;
 
 import org.apache.log4j.Logger;
 import org.apache.zookeeper.PortAssignment;
-import org.apache.zookeeper.jmx.MBeanRegistry;
 import org.apache.zookeeper.server.NIOServerCnxn;
 import org.apache.zookeeper.server.quorum.Election;
 import org.apache.zookeeper.server.quorum.LeaderElection;
-import org.apache.zookeeper.server.quorum.LeaderElectionBean;
 import org.apache.zookeeper.server.quorum.QuorumPeer;
 import org.apache.zookeeper.server.quorum.Vote;
-import org.apache.zookeeper.server.quorum.LeaderElection.ElectionResult;
 import org.apache.zookeeper.server.quorum.QuorumPeer.LearnerType;
 import org.apache.zookeeper.server.quorum.QuorumPeer.QuorumServer;
 import org.apache.zookeeper.server.quorum.QuorumPeer.ServerState;

src/java/test/org/apache/zookeeper/test/ObserverHierarchicalQuorumTest.java

@@ -37,19 +37,7 @@ public class ObserverHierarchicalQuorumTest extends HierarchicalQuorumTest {
     }
            
     protected void shutdown(QuorumPeer qp) {
-        try {
-            /* TODO: when Observers are compatible with fle, shutdown
-             * the leader election */            
-            LOG.info("Done with leader election");
-            qp.shutdown();
-            LOG.info("Done with quorum peer");
-            qp.join(30000);
-            if (qp.isAlive()) {
-                fail("QP failed to shutdown in 30 seconds");
-            }
-        } catch (InterruptedException e) {
-            LOG.debug("QP interrupted", e);
-        }
+        QuorumBase.shutdown(qp);
     }
 
     @Test

src/java/test/org/apache/zookeeper/test/ObserverQuorumHammerTest.java

@@ -17,7 +17,6 @@
  */
 
 package org.apache.zookeeper.test;
-import org.apache.log4j.Logger;
 import org.junit.Before;
 import org.junit.Test;
 

src/java/test/org/apache/zookeeper/test/ObserverTest.java

@@ -20,8 +20,6 @@ package org.apache.zookeeper.test;
 
 import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT;
 
-import java.io.File;
-import java.io.FileWriter;
 import java.io.IOException;
 import java.util.concurrent.CountDownLatch;
 
@@ -35,9 +33,7 @@ import org.apache.zookeeper.KeeperException.ConnectionLossException;
 import org.apache.zookeeper.Watcher.Event.KeeperState;
 import org.apache.zookeeper.ZooDefs.Ids;
 import org.apache.zookeeper.ZooKeeper.States;
-import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
 import org.apache.zookeeper.server.quorum.QuorumPeerTestBase;
-import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException;
 import org.junit.Test;
 
 /**

src/java/test/org/apache/zookeeper/test/QuorumBase.java

@@ -30,7 +30,7 @@ import java.util.Set;
 import org.apache.log4j.Logger;
 import org.apache.zookeeper.PortAssignment;
 import org.apache.zookeeper.TestableZooKeeper;
-import org.apache.zookeeper.server.quorum.FastLeaderElection;
+import org.apache.zookeeper.server.quorum.Election;
 import org.apache.zookeeper.server.quorum.QuorumPeer;
 import org.apache.zookeeper.server.quorum.QuorumPeer.LearnerType;
 import org.apache.zookeeper.server.quorum.QuorumPeer.QuorumServer;
@@ -284,16 +284,24 @@ public class QuorumBase extends ClientBase {
         shutdown(s5);
     }
 
-    protected void shutdown(QuorumPeer qp) {
+    public static void shutdown(QuorumPeer qp) {
         try {
+            LOG.info("Shutting down quorum peer " + qp.getName());
             qp.shutdown();
-            ((FastLeaderElection) qp.getElectionAlg()).shutdown();
+            Election e = qp.getElectionAlg();
+            if (e != null) {
+                LOG.info("Shutting down leader election " + qp.getName());
+                e.shutdown();
+            } else {
+                LOG.info("No election available to shutdown " + qp.getName());
+            }
+            LOG.info("Waiting for " + qp.getName() + " to exit thread");
             qp.join(30000);
             if (qp.isAlive()) {
-                fail("QP failed to shutdown in 30 seconds");
+                fail("QP failed to shutdown in 30 seconds: " + qp.getName());
             }
         } catch (InterruptedException e) {
-            LOG.debug("QP interrupted", e);
+            LOG.debug("QP interrupted: " + qp.getName(), e);
         }
     }
 

src/java/test/org/apache/zookeeper/test/QuorumTest.java

@@ -30,8 +30,8 @@ import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.ZooDefs.Ids;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.quorum.LearnerHandler;
 import org.apache.zookeeper.server.quorum.Leader;
+import org.apache.zookeeper.server.quorum.LearnerHandler;
 import org.junit.Before;
 import org.junit.Test;
 

src/java/test/org/apache/zookeeper/test/TruncateTest.java

@@ -132,11 +132,8 @@ public class TruncateTest extends TestCase {
             // this is what we want
         }
         zk1.close();
-        s1.shutdown();
-        s1.join();
-        s2.shutdown();
-        s2.join();
-        s3.shutdown();
-        s3.join();
+        QuorumBase.shutdown(s1);
+        QuorumBase.shutdown(s2);
+        QuorumBase.shutdown(s3);
     }
 }

src/java/test/org/apache/zookeeper/test/ZkDatabaseCorruptionTest.java

@@ -19,13 +19,8 @@
 package org.apache.zookeeper.test;
 
 import java.io.File;
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.RandomAccessFile;
-import java.net.InetSocketAddress;
-import java.util.HashMap;
-import java.util.LinkedHashSet;
-import java.util.Set;
 
 import org.apache.log4j.Logger;
 import org.apache.zookeeper.CreateMode;
@@ -36,8 +31,6 @@ import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.server.SyncRequestProcessor;
 import org.apache.zookeeper.server.persistence.FileTxnSnapLog;
 import org.apache.zookeeper.server.quorum.QuorumPeer;
-import org.apache.zookeeper.server.quorum.QuorumPeer.LearnerType;
-import org.apache.zookeeper.server.quorum.QuorumPeer.QuorumServer;
 import org.apache.zookeeper.server.quorum.QuorumPeer.ServerState;
 import org.junit.Before;
 
@@ -122,10 +115,10 @@ public class ZkDatabaseCorruptionTest extends QuorumBase {
             zk.create("/0-" + i, new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
         }
         zk.close();
-        qb.s1.shutdown();
-        qb.s2.shutdown();
-        qb.s3.shutdown();
-        qb.s4.shutdown();
+        QuorumBase.shutdown(qb.s1);
+        QuorumBase.shutdown(qb.s2);
+        QuorumBase.shutdown(qb.s3);
+        QuorumBase.shutdown(qb.s4);
     }