Browse Source

ZOOKEEPER-408. address all findbugs warnings in persistence classes. (phunt, mahadev, flavio via mahadev)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/zookeeper/trunk@786156 13f79535-47bb-0310-9956-ffa450edef68
Mahadev Konar 16 năm trước cách đây
mục cha
commit
3402537c1f
38 tập tin đã thay đổi với 534 bổ sung387 xóa
  1. 3 0
      CHANGES.txt
  2. 18 4
      src/java/main/org/apache/zookeeper/ClientCnxn.java
  3. 1 1
      src/java/main/org/apache/zookeeper/Environment.java
  4. 1 1
      src/java/main/org/apache/zookeeper/ZooKeeper.java
  5. 5 2
      src/java/main/org/apache/zookeeper/ZooKeeperMain.java
  6. 7 5
      src/java/main/org/apache/zookeeper/server/DataNode.java
  7. 47 11
      src/java/main/org/apache/zookeeper/server/DataTree.java
  8. 10 2
      src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java
  9. 18 4
      src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
  10. 0 2
      src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java
  11. 19 4
      src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java
  12. 1 2
      src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
  13. 89 83
      src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
  14. 9 3
      src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
  15. 17 2
      src/java/main/org/apache/zookeeper/server/persistence/Util.java
  16. 1 4
      src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java
  17. 5 1
      src/java/main/org/apache/zookeeper/server/quorum/FollowerHandler.java
  18. 2 1
      src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java
  19. 7 1
      src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java
  20. 7 12
      src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
  21. 5 1
      src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
  22. 43 0
      src/java/test/config/findbugsExcludeFile.xml
  23. 1 1
      src/java/test/org/apache/zookeeper/server/CRCTest.java
  24. 2 2
      src/java/test/org/apache/zookeeper/server/InvalidSnapshotTest.java
  25. 16 16
      src/java/test/org/apache/zookeeper/test/ACLTest.java
  26. 9 9
      src/java/test/org/apache/zookeeper/test/AsyncOpsTest.java
  27. 15 16
      src/java/test/org/apache/zookeeper/test/ClientBase.java
  28. 2 2
      src/java/test/org/apache/zookeeper/test/ClientTest.java
  29. 62 63
      src/java/test/org/apache/zookeeper/test/FLENewEpochTest.java
  30. 55 58
      src/java/test/org/apache/zookeeper/test/FLETest.java
  31. 32 46
      src/java/test/org/apache/zookeeper/test/HierarchicalQuorumTest.java
  32. 1 1
      src/java/test/org/apache/zookeeper/test/InvalidSnapshotTest.java
  33. 4 4
      src/java/test/org/apache/zookeeper/test/LETest.java
  34. 1 1
      src/java/test/org/apache/zookeeper/test/PurgeTxnTest.java
  35. 5 8
      src/java/test/org/apache/zookeeper/test/QuorumBase.java
  36. 3 3
      src/java/test/org/apache/zookeeper/test/RecoveryTest.java
  37. 1 1
      src/java/test/org/apache/zookeeper/test/UpgradeTest.java
  38. 10 10
      src/java/test/org/apache/zookeeper/test/WatcherTest.java

+ 3 - 0
CHANGES.txt

@@ -121,6 +121,9 @@ BUGFIXES:
 
   ZOOKEEPER-433.  getacl on root znode (/) fails. (phunt via mahadev)
 
+  ZOOKEEPER-408. address all findbugs warnings in persistence classes.
+(phunt, mahadev, flavio via mahadev)
+
 IMPROVEMENTS:
   ZOOKEEPER-308. improve the atomic broadcast performance 3x.
   (breed via mahadev)

+ 18 - 4
src/java/main/org/apache/zookeeper/ClientCnxn.java

@@ -80,9 +80,9 @@ public class ClientCnxn {
      * Clients automatically reset watches during session reconnect, this
      * option allows the client to turn off this behavior by setting
      * the environment variable "zookeeper.disableAutoWatchReset" to "true" */
-    public static boolean disableAutoWatchReset;
+    private static boolean disableAutoWatchReset;
    
-    public static int packetLen;
+    public static final int packetLen;
     static {
         // this var should not be public, but otw there is no easy way 
         // to test
@@ -104,7 +104,7 @@ public class ClientCnxn {
 
         byte data[];
     }
-
+    
     private ArrayList<AuthData> authInfo = new ArrayList<AuthData>();
 
     /**
@@ -305,7 +305,21 @@ public class ClientCnxn {
         sendThread = new SendThread();
         eventThread = new EventThread();
     }
-
+    
+    /**
+     * tests use this to check on reset of watches
+     * @return if the auto reset of watches are disabled
+     */
+    public static boolean getDisableAutoResetWatch() {
+        return disableAutoWatchReset;
+    }
+    /**
+     * tests use this to set the auto reset
+     * @param b the vaued to set disable watches to
+     */
+    public static void setDisableAutoResetWatch(boolean b) {
+        disableAutoWatchReset = b;
+    }
     public void start() {
         sendThread.start();
         eventThread.start();

+ 1 - 1
src/java/main/org/apache/zookeeper/Environment.java

@@ -56,7 +56,7 @@ public class Environment {
 
         try {
             put(l, "host.name",
-                InetAddress.getLocalHost().getCanonicalHostName().toString());
+                InetAddress.getLocalHost().getCanonicalHostName());
         } catch (UnknownHostException e) {
             put(l, "host.name", "<NA>");
         }

+ 1 - 1
src/java/main/org/apache/zookeeper/ZooKeeper.java

@@ -165,7 +165,7 @@ public class ZooKeeper {
                 }
 
                 // clear the watches if auto watch reset is not enabled
-                if (ClientCnxn.disableAutoWatchReset &&
+                if (ClientCnxn.getDisableAutoResetWatch() &&
                         state != Watcher.Event.KeeperState.SyncConnected)
                 {
                     synchronized(dataWatches) {

+ 5 - 2
src/java/main/org/apache/zookeeper/ZooKeeperMain.java

@@ -46,7 +46,7 @@ import org.apache.zookeeper.data.Stat;
  */
 public class ZooKeeperMain {
     private static final Logger LOG = Logger.getLogger(ZooKeeperMain.class);
-    protected static Map<String,String> commandMap = new HashMap<String,String>( );
+    protected static final Map<String,String> commandMap = new HashMap<String,String>( );
 
     protected MyCommandOptions cl = new MyCommandOptions();
     protected HashMap<Integer,String> history = new HashMap<Integer,String>( );
@@ -538,8 +538,11 @@ public class ZooKeeperMain {
         // now create the direct children
         // and the stat and quota nodes
         String[] splits = path.split("/");
+        StringBuffer sb = new StringBuffer();
+        sb.append(quotaPath);
         for (int i=1; i<splits.length; i++) {
-            quotaPath = quotaPath + "/" + splits[i];
+            sb.append("/" + splits[i]);
+            quotaPath = sb.toString();
             try {
                 zk.create(quotaPath, null, Ids.OPEN_ACL_UNSAFE ,
                         CreateMode.PERSISTENT);

+ 7 - 5
src/java/main/org/apache/zookeeper/server/DataNode.java

@@ -53,7 +53,9 @@ public class DataNode implements Record {
     /** the list of children for this node. note 
      * that the list of children string does not 
      * contain the parent path -- just the last
-     * part of the path.
+     * part of the path. This should be 
+     * synchronized on except deserializing 
+     * (for speed up issues).
      */
     Set<String> children = new HashSet<String>();
 
@@ -83,7 +85,7 @@ public class DataNode implements Record {
      * fully
      * @param children
      */
-    public void setChildren(HashSet<String> children) {
+    public synchronized void setChildren(HashSet<String> children) {
         this.children = children;
     }
     
@@ -91,12 +93,12 @@ public class DataNode implements Record {
      * convenience methods to get the children
      * @return the children of this datanode
      */
-    public Set<String> getChildren() {
+    public synchronized Set<String> getChildren() {
         return this.children;
     }
     
    
-    public void copyStat(Stat to) {
+    synchronized public void copyStat(Stat to) {
         to.setAversion(stat.getAversion());
         to.setCtime(stat.getCtime());
         to.setCversion(stat.getCversion());
@@ -110,7 +112,7 @@ public class DataNode implements Record {
         to.setNumChildren(children.size());
     }
 
-    public void deserialize(InputArchive archive, String tag)
+    synchronized public void deserialize(InputArchive archive, String tag)
             throws IOException {
         archive.startRecord("node");
         data = archive.readBuffer("data");

+ 47 - 11
src/java/main/org/apache/zookeeper/server/DataTree.java

@@ -82,20 +82,20 @@ public class DataTree {
     private WatchManager childWatches = new WatchManager();
 
     /** the root of zookeeper tree */
-    private final String rootZookeeper = "/";
+    private static final String rootZookeeper = "/";
     
     /** the zookeeper nodes that acts as the management and status node **/
-    private final String procZookeeper = Quotas.procZookeeper;
+    private static final String procZookeeper = Quotas.procZookeeper;
     
     /** this will be the string thats stored as a child of root */
-    private final String procChildZookeeper = procZookeeper.substring(1);
+    private static final String procChildZookeeper = procZookeeper.substring(1);
     
     /** the zookeeper quota node that acts as the quota 
      * management node for zookeeper */
-    private final String quotaZookeeper = Quotas.quotaZookeeper;
+    private static final String quotaZookeeper = Quotas.quotaZookeeper;
 
     /** this will be the string thats stored as a child of /zookeeper */
-    private final String quotaChildZookeeper = quotaZookeeper.substring(
+    private static final String quotaChildZookeeper = quotaZookeeper.substring(
             procZookeeper.length() + 1);
     
     /** 
@@ -249,8 +249,10 @@ public class DataTree {
     public long approximateDataSize() {
         long result = 0;
         for (Map.Entry<String, DataNode> entry : nodes.entrySet()) {
-            result += entry.getKey().length();
-            result += entry.getValue().data.length;
+            synchronized(entry.getValue()) {
+                result += entry.getKey().length();
+                result += (entry.getValue().data == null? 0 : entry.getValue().data.length);
+            }
         }
         return result;
     }
@@ -340,6 +342,11 @@ public class DataTree {
         String statNode = Quotas.statPath(lastPrefix);
         DataNode node = nodes.get(statNode);
         StatsTrack updatedStat = null;
+        if (node == null) {
+            //should not happen
+            LOG.error("Missing count node for stat " + statNode);
+            return;
+        }
         synchronized(node) {
             updatedStat = new StatsTrack(new String(node.data));
             updatedStat.setCount(updatedStat.getCount() + diff);
@@ -349,6 +356,11 @@ public class DataTree {
         String quotaNode = Quotas.quotaPath(lastPrefix);
         node = nodes.get(quotaNode);
         StatsTrack thisStats = null;
+        if (node == null) {
+            //should not happen
+            LOG.error("Missing count node for quota " + quotaNode);
+            return;
+        }
         synchronized(node) {
             thisStats = new StatsTrack(new String(node.data));
         }
@@ -363,10 +375,17 @@ public class DataTree {
      * update the count of bytes of this stat datanode
      * @param lastPrefix the path of the node that is quotaed
      * @param diff the diff to added to number of bytes
+     * @throws IOException if path is not found
      */
-    public void updateBytes(String lastPrefix, long diff) {
+    public void updateBytes(String lastPrefix, long diff)  {
         String statNode = Quotas.statPath(lastPrefix);
         DataNode node = nodes.get(statNode);
+        if (node == null) {
+            //should never be null but just to make 
+            // findbugs happy
+            LOG.error("Missing stat node for bytes " + statNode);
+            return;
+        }
         StatsTrack updatedStat = null;
         synchronized(node) {
             updatedStat = new StatsTrack(new String(node.data));
@@ -376,6 +395,12 @@ public class DataTree {
         // now check if the bytes match the quota
         String quotaNode = Quotas.quotaPath(lastPrefix);
         node = nodes.get(quotaNode);
+        if (node == null) {
+            //should never be null but just to make
+            // findbugs happy
+            LOG.error("Missing quota node for bytes " + quotaNode);
+            return;
+        }
         StatsTrack thisStats = null;
         synchronized(node) {
             thisStats = new StatsTrack(new String(node.data));
@@ -515,7 +540,11 @@ public class DataTree {
         if (!rootZookeeper.equals(lastPrefix) && !("".equals(lastPrefix))) {
             // ok we have some match and need to update 
             updateCount(lastPrefix, -1);
-            updateBytes(lastPrefix, node.data == null? 0:-(node.data.length));
+            int bytes = 0;
+            synchronized (node) {
+                bytes = (node.data == null? 0:-(node.data.length));
+            }
+            updateBytes(lastPrefix, bytes);
         }
         if (LOG.isTraceEnabled()) {
             ZooTrace.logTraceMessage(LOG,
@@ -538,8 +567,9 @@ public class DataTree {
         if (n == null) {
             throw new KeeperException.NoNodeException();
         }
-        byte lastdata[] = n.data;
+        byte lastdata[] = null;
         synchronized (n) {
+            lastdata = n.data;
             n.data = data;
             n.stat.setMtime(time);
             n.stat.setMzxid(zxid);
@@ -782,13 +812,15 @@ public class DataTree {
             return;
         }
         String[] children = null;
+        int len = 0;
         synchronized (node) {
             children = node.children.toArray(new
                     String[node.children.size()]);
+            len = (node.data == null? 0: node.data.length);
         }
         // add itself
         counts.count += 1;
-        counts.bytes += (long)node.data.length;
+        counts.bytes += len;
         if (children.length == 0) {
             return;
         }
@@ -966,6 +998,10 @@ public class DataTree {
             } else {
                 String parentPath = path.substring(0, lastSlash);
                 node.parent = nodes.get(parentPath);
+                if (node.parent == null) {
+                    throw new IOException("Invalid Datatree, unable to find " +
+                    		"parent " + parentPath + " of path " + path);
+                }
                 node.parent.children.add(path.substring(lastSlash + 1));
                 long eowner = node.stat.getEphemeralOwner();
                 if (eowner != 0) {

+ 10 - 2
src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java

@@ -194,7 +194,11 @@ public class FinalRequestProcessor implements RequestProcessor {
                 if (n == null) {
                     throw new KeeperException.NoNodeException();
                 }
-                PrepRequestProcessor.checkACL(zks, zks.dataTree.convertLong(n.acl),
+                Long aclL;
+                synchronized(n) {
+                    aclL = n.acl;
+                }
+                PrepRequestProcessor.checkACL(zks, zks.dataTree.convertLong(aclL),
                         ZooDefs.Perms.READ,
                         request.authInfo);
                 stat = new Stat();
@@ -231,7 +235,11 @@ public class FinalRequestProcessor implements RequestProcessor {
                 if (n == null) {
                     throw new KeeperException.NoNodeException();
                 }
-                PrepRequestProcessor.checkACL(zks, zks.dataTree.convertLong(n.acl), 
+                Long aclG;
+                synchronized(n) {
+                    aclG = n.acl;
+                }
+                PrepRequestProcessor.checkACL(zks, zks.dataTree.convertLong(aclG), 
                         ZooDefs.Perms.READ,
                         request.authInfo);
                 List<String> children = zks.dataTree.getChildren(

+ 18 - 4
src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java

@@ -23,6 +23,7 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.LinkedBlockingQueue;
 
 import org.apache.jute.Record;
@@ -73,7 +74,7 @@ public class PrepRequestProcessor extends Thread implements RequestProcessor {
      * this is only for testing purposes.
      * should never be useed otherwise
      */
-    public static boolean failCreate = false;
+    private static  boolean failCreate = false;
     
     LinkedBlockingQueue<Request> submittedRequests = new LinkedBlockingQueue<Request>();
 
@@ -87,7 +88,14 @@ public class PrepRequestProcessor extends Thread implements RequestProcessor {
         this.nextProcessor = nextProcessor;
         this.zks = zks;
     }
-
+    
+    /**
+     * method for tests to set failCreate
+     * @param b
+     */
+    public static void setFailCreate(boolean b) {
+        failCreate = b;
+    }
     @Override
     public void run() {
         try {
@@ -126,8 +134,14 @@ public class PrepRequestProcessor extends Thread implements RequestProcessor {
             if (lastChange == null) {
                 DataNode n = zks.dataTree.getNode(path);
                 if (n != null) {
-                    lastChange = new ChangeRecord(-1, path, n.stat, n.children
-                            .size(), zks.dataTree.convertLong(n.acl));
+                    Long acl;
+                    Set<String> children;
+                    synchronized(n) {
+                        acl = n.acl;
+                        children = n.children;
+                    }
+                    lastChange = new ChangeRecord(-1, path, n.stat, children
+                            .size(), zks.dataTree.convertLong(acl));
                 }
             }
         }

+ 0 - 2
src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java

@@ -44,7 +44,6 @@ public class SessionTrackerImpl extends Thread implements SessionTracker {
 
     ConcurrentHashMap<Long, Integer> sessionsWithTimeout;
     long nextSessionId = 0;
-    long serverId;
     long nextExpirationTime;
 
     int expirationInterval;
@@ -87,7 +86,6 @@ public class SessionTrackerImpl extends Thread implements SessionTracker {
         this.expirationInterval = tickTime;
         this.sessionsWithTimeout = sessionsWithTimeout;
         nextExpirationTime = roundToInterval(System.currentTimeMillis());
-        this.serverId = sid;
         this.nextSessionId = initializeNextSession(sid);
         for (Entry<Long, Integer> e : sessionsWithTimeout.entrySet()) {
             addSession(e.getKey(), e.getValue());

+ 19 - 4
src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java

@@ -37,7 +37,6 @@ public class SyncRequestProcessor extends Thread implements RequestProcessor {
     private ZooKeeperServer zks;
     private LinkedBlockingQueue<Request> queuedRequests = new LinkedBlockingQueue<Request>();
     private RequestProcessor nextProcessor;
-    boolean timeToDie = false;
     Thread snapInProcess = null;
     
     /**
@@ -51,7 +50,7 @@ public class SyncRequestProcessor extends Thread implements RequestProcessor {
     /**
      * The number of log entries to log before starting a snapshot
      */
-    public static int snapCount = ZooKeeperServer.getSnapCount();
+    private static int snapCount = ZooKeeperServer.getSnapCount();
 
     private Request requestOfDeath = Request.requestOfDeath;
 
@@ -61,7 +60,24 @@ public class SyncRequestProcessor extends Thread implements RequestProcessor {
         this.zks = zks;
         this.nextProcessor = nextProcessor;
     }
-
+    
+    /**
+     * used by tests to check for changing 
+     * snapcounts
+     * @param count
+     */
+    public static void setSnapCount(int count) {
+        snapCount = count;
+    }
+    
+    /**
+     * used by tests to get the snapcount
+     * @return the snapcount
+     */
+    public static int getSnapCount() {
+        return snapCount;
+    }
+    
     @Override
     public void run() {
         try {
@@ -133,7 +149,6 @@ public class SyncRequestProcessor extends Thread implements RequestProcessor {
     }
 
     public void shutdown() {
-        timeToDie = true;
         queuedRequests.add(requestOfDeath);
         nextProcessor.shutdown();
     }

+ 1 - 2
src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java

@@ -108,14 +108,13 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider {
     protected long hzxid = 0;
     final public static Exception ok = new Exception("No prob");
     protected RequestProcessor firstProcessor;
-    LinkedBlockingQueue<Long> sessionsToDie = new LinkedBlockingQueue<Long>();
     protected volatile boolean running;
 
     /**
      * This is the secret that we use to generate passwords, for the moment it
      * is more of a sanity check.
      */
-    final private long superSecret = 0XB3415C00L;
+    static final private long superSecret = 0XB3415C00L;
 
     int requestsInProcess;
     List<ChangeRecord> outstandingChanges = new ArrayList<ChangeRecord>();

+ 89 - 83
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java

@@ -49,25 +49,44 @@ import org.apache.zookeeper.txn.TxnHeader;
  *
  */
 public class FileTxnLog implements TxnLog {
+    private static final Logger LOG;
+
+    static long preAllocSize =  65536 * 1024;
+
+    public final static int TXNLOG_MAGIC =
+        ByteBuffer.wrap("ZKLG".getBytes()).getInt();
+
+    public final static int VERSION = 2;
+
+    static {
+        LOG = Logger.getLogger(FileTxnLog.class);
+
+        forceSync =
+            !System.getProperty("zookeeper.forceSync", "yes").equals("no");
+
+        String size = System.getProperty("zookeeper.preAllocSize");
+        if (size != null) {
+            try {
+                preAllocSize = Long.parseLong(size) * 1024;
+            } catch (NumberFormatException e) {
+                LOG.warn(size + " is not a valid value for preAllocSize");
+            }
+        }
+    }
+
     long lastZxidSeen;
     volatile BufferedOutputStream logStream = null;
     volatile OutputArchive oa;
     volatile FileOutputStream fos = null;
-    
-    
+
     File logDir;
-    public final static int TXNLOG_MAGIC =
-        ByteBuffer.wrap("ZKLG".getBytes()).getInt();
-    public final static int VERSION = 2;
-    private boolean forceSync = true;
+    private static boolean forceSync = true;
     long dbId;
-    private LinkedList<FileOutputStream> streamsToFlush = new LinkedList<FileOutputStream>();
-    static long preAllocSize =  65536 * 1024; 
+    private LinkedList<FileOutputStream> streamsToFlush =
+        new LinkedList<FileOutputStream>();
     long currentSize;
     File logFileWrite = null;
-    
-    private static final Logger LOG = Logger.getLogger(FileTxnLog.class);
-  
+
     /**
      * constructor for FileTxnLog. Take the directory
      * where the txnlogs are stored
@@ -75,27 +94,17 @@ public class FileTxnLog implements TxnLog {
      */
     public FileTxnLog(File logDir) {
         this.logDir = logDir;
-        forceSync = !System.getProperty("zookeeper.forceSync", "yes").equals(
-            "no");
-        String size = System.getProperty("zookeeper.preAllocSize");
-        if (size != null) {
-            try {
-                preAllocSize = Long.parseLong(size) * 1024;
-            } catch (NumberFormatException e) {
-                LOG.warn(size + " is not a valid value for preAllocSize");
-            }
-        }
     }
-    
+
     /**
      * method to allow setting preallocate size
      * of log file to pad the file.
-     * @param size the size to set to
+     * @param size the size to set to in bytes
      */
     public static void setPreallocSize(long size) {
         preAllocSize = size;
     }
-    
+
     /**
      * creates a checksum alogrithm to be used
      * @return the checksum used for this txnlog
@@ -107,7 +116,7 @@ public class FileTxnLog implements TxnLog {
 
     /**
      * rollover the current log file to a new one.
-     * @throws IOException 
+     * @throws IOException
      */
     public void rollLog() throws IOException {
         if (logStream != null) {
@@ -122,7 +131,7 @@ public class FileTxnLog implements TxnLog {
      * @param hdr the header of the transaction
      * @param txn the transaction part of the entry
      */
-    public synchronized void append(TxnHeader hdr, Record txn) 
+    public synchronized void append(TxnHeader hdr, Record txn)
         throws IOException {
         if (hdr != null) {
             if (hdr.getZxid() <= lastZxidSeen) {
@@ -131,7 +140,7 @@ public class FileTxnLog implements TxnLog {
                         + hdr.getType());
             }
             if (logStream==null) {
-               logFileWrite = new File(logDir, ("log." + 
+               logFileWrite = new File(logDir, ("log." +
                        Long.toHexString(hdr.getZxid())));
                fos = new FileOutputStream(logFileWrite);
                logStream=new BufferedOutputStream(fos);
@@ -145,15 +154,15 @@ public class FileTxnLog implements TxnLog {
             byte[] buf = Util.marshallTxnEntry(hdr, txn);
             if (buf == null || buf.length == 0) {
                 throw new IOException("Faulty serialization for header " +
-                		"and txn");
+                        "and txn");
             }
             Checksum crc = makeChecksumAlgorithm();
             crc.update(buf, 0, buf.length);
             oa.writeLong(crc.getValue(), "txnEntryCRC");
             Util.writeTxnBytes(oa, buf);
-        }    
+        }
     }
-    
+
     /**
      * pad the current file to increase its size
      * @param out the outputstream to be padded
@@ -162,7 +171,7 @@ public class FileTxnLog implements TxnLog {
     private void padFile(FileOutputStream out) throws IOException {
         currentSize = Util.padLogFile(out, currentSize, preAllocSize);
     }
-    
+
     /**
      * Find the log file that starts at, or just before, the snapshot. Return
      * this and all subsequent logs. Results are ordered by zxid of file,
@@ -181,7 +190,7 @@ public class FileTxnLog implements TxnLog {
             if (fzxid > snapshotZxid) {
                 continue;
             }
-            // the files 
+            // the files
             // are sorted with zxid's
             if (fzxid > logZxid) {
                 logZxid = fzxid;
@@ -196,9 +205,9 @@ public class FileTxnLog implements TxnLog {
             v.add(f);
         }
         return v.toArray(new File[0]);
-    
+
     }
-    
+
     /**
      * get the last zxid that was logged in the transaction logs
      * @return the last zxid logged in the transaction logs
@@ -207,11 +216,10 @@ public class FileTxnLog implements TxnLog {
         File[] files = getLogFiles(logDir.listFiles(), 0);
         long maxLog=files.length>0?
                 Util.getZxidFromName(files[files.length-1].getName(),"log"):-1;
-        
-        // if a log file is more recent we must scan it to find 
+
+        // if a log file is more recent we must scan it to find
         // the highest zxid
         long zxid = maxLog;
-        FileOutputStream logStream = null;
         try {
             FileTxnLog txn = new FileTxnLog(logDir);
             TxnIterator itr = txn.read(maxLog);
@@ -223,17 +231,12 @@ public class FileTxnLog implements TxnLog {
             }
         } catch (IOException e) {
             LOG.warn("Unexpected exception", e);
-        } finally {
-            if (logStream != null)
-                try {
-                    logStream.close();
-                } catch(IOException io){}
         }
         return zxid;
     }
-    
+
     /**
-     * commit the logs. make sure that evertyhing hits the 
+     * commit the logs. make sure that evertyhing hits the
      * disk
      */
     public synchronized void commit() throws IOException {
@@ -250,7 +253,7 @@ public class FileTxnLog implements TxnLog {
             streamsToFlush.removeFirst().close();
         }
     }
-    
+
     /**
      * start reading all the transactions from the given zxid
      * @param zxid the zxid to start reading transactions from
@@ -259,8 +262,8 @@ public class FileTxnLog implements TxnLog {
      */
     public TxnIterator read(long zxid) throws IOException {
         return new FileTxnIterator(logDir, zxid);
-    }   
-    
+    }
+
     /**
      * truncate the current transaction logs
      * @param zxid the zxid to truncate the logs to
@@ -275,11 +278,13 @@ public class FileTxnLog implements TxnLog {
         raf.setLength(pos);
         raf.close();
         while(itr.goToNextLog()) {
-            itr.logFile.delete();
+            if (!itr.logFile.delete()) {
+                LOG.warn("Unable to truncate " + itr.logFile);
+            }
         }
         return true;
     }
-    
+
     /**
      * read the header of the transaction file
      * @param file the transaction file to read
@@ -288,20 +293,21 @@ public class FileTxnLog implements TxnLog {
      */
     private static FileHeader readHeader(File file) throws IOException {
         InputStream is =null;
-        try{
+        try {
             is = new BufferedInputStream(new FileInputStream(file));
             InputArchive ia=BinaryInputArchive.getArchive(is);
             FileHeader hdr = new FileHeader();
             hdr.deserialize(ia, "fileheader");
             return hdr;
-         }finally{
-             try{
-                 if(is != null) is.close();
-             }catch(IOException e){
+         } finally {
+             try {
+                 if (is != null) is.close();
+             } catch (IOException e) {
+                 LOG.warn("Ignoring exception during close", e);
              }
-         }        
+         }
     }
-    
+
     /**
      * the dbid of this transaction database
      * @return the dbid of this database
@@ -314,10 +320,10 @@ public class FileTxnLog implements TxnLog {
             throw new IOException("Unsupported Format.");
         return fh.getDbid();
     }
-    
+
     /**
-     * this class implements the txnlog iterator interface 
-     * which is used for reading the transaction logs 
+     * this class implements the txnlog iterator interface
+     * which is used for reading the transaction logs
      */
     public static class FileTxnIterator implements TxnLog.TxnIterator {
         File logDir;
@@ -328,10 +334,10 @@ public class FileTxnLog implements TxnLog {
         InputArchive ia;
         static final String CRC_ERROR="CRC check failed";
         FileInputStream inputStream=null;
-        //stored files is the list of files greater than 
+        //stored files is the list of files greater than
         //the zxid we are looking for.
         private ArrayList<File> storedFiles;
-        
+
         /**
          * create an iterator over a transaction database directory
          * @param logDir the transaction database directory
@@ -343,10 +349,10 @@ public class FileTxnLog implements TxnLog {
           this.zxid = zxid;
           init();
         }
-        
+
         /**
          * initialize to the zxid specified
-         * this is inclusive of the zxid 
+         * this is inclusive of the zxid
          * @throws IOException
          */
         void init() throws IOException {
@@ -370,10 +376,10 @@ public class FileTxnLog implements TxnLog {
                     return;
             }
         }
-        
+
         /**
-         * go to the next logfile 
-         * @return true if there is one and false if there is no 
+         * go to the next logfile
+         * @return true if there is one and false if there is no
          * new file to be read
          * @throws IOException
          */
@@ -385,23 +391,23 @@ public class FileTxnLog implements TxnLog {
             }
             return false;
         }
-        
+
         /**
          * read the header fomr the inputarchive
          * @param ia the inputarchive to be read from
-         * @param is the inputstream 
+         * @param is the inputstream
          * @throws IOException
          */
-        protected void inStreamCreated(InputArchive ia, FileInputStream is) 
+        protected void inStreamCreated(InputArchive ia, FileInputStream is)
             throws IOException{
             FileHeader header= new FileHeader();
             header.deserialize(ia, "fileheader");
             if (header.getMagic() != FileTxnLog.TXNLOG_MAGIC) {
-                throw new IOException("Invalid magic number " + header.getMagic() 
+                throw new IOException("Invalid magic number " + header.getMagic()
                         + " != " + FileTxnLog.TXNLOG_MAGIC);
-            }  
+            }
         }
-        
+
         /**
          * Invoked to indicate that the input stream has been created.
          * @param ia input archive
@@ -418,15 +424,15 @@ public class FileTxnLog implements TxnLog {
             }
             return ia;
         }
-        
+
         /**
-         * create a checksum algorithm 
+         * create a checksum algorithm
          * @return the checksum algorithm
          */
         protected Checksum makeChecksumAlgorithm(){
             return new Adler32();
         }
-        
+
         /**
          * the iterator that moves to the next transaction
          * @return true if there is more transactions to be read
@@ -441,12 +447,12 @@ public class FileTxnLog implements TxnLog {
                 byte[] bytes = Util.readTxnBytes(ia);
                 // Since we preallocate, we define EOF to be an
                 if (bytes == null || bytes.length==0)
-                   throw new EOFException("Failed to read"); 
+                   throw new EOFException("Failed to read");
                 // EOF or corrupted record
                 // validate CRC
                 Checksum crc = makeChecksumAlgorithm();
                 crc.update(bytes, 0, bytes.length);
-                if (crcValue != crc.getValue()) 
+                if (crcValue != crc.getValue())
                     throw new IOException(CRC_ERROR);
                 if (bytes == null || bytes.length == 0)
                     return false;
@@ -459,7 +465,7 @@ public class FileTxnLog implements TxnLog {
                 inputStream.close();
                 inputStream = null;
                 ia = null;
-                // thsi means that the file has ended 
+                // thsi means that the file has ended
                 // we shoud go to the next file
                 if (!goToNextLog()) {
                     return false;
@@ -467,10 +473,10 @@ public class FileTxnLog implements TxnLog {
             }
             return true;
         }
-        
+
         /**
-         * reutrn the current header 
-         * @return the current header that 
+         * reutrn the current header
+         * @return the current header that
          * is read
          */
         public TxnHeader getHeader() {
@@ -485,9 +491,9 @@ public class FileTxnLog implements TxnLog {
         public Record getTxn() {
             return record;
         }
-        
+
         /**
-         * close the iterator 
+         * close the iterator
          * and release the resources.
          */
         public void close() throws IOException {

+ 9 - 3
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java

@@ -71,14 +71,20 @@ public class FileTxnSnapLog {
      * @param dataDir the trasaction directory
      * @param snapDir the snapshot directory
      */
-    public FileTxnSnapLog(File dataDir, File snapDir) {
+    public FileTxnSnapLog(File dataDir, File snapDir) throws IOException {
         this.dataDir = new File(dataDir, version + VERSION);
         this.snapDir = new File(snapDir, version + VERSION);
         if (!this.dataDir.exists()) {
-            this.dataDir.mkdirs();
+            if (!this.dataDir.mkdirs()) {
+                throw new IOException("Unable to create data directory "
+                        + this.dataDir);
+            }
         }
         if (!this.snapDir.exists()) {
-            this.snapDir.mkdirs();
+            if (!this.snapDir.mkdirs()) {
+                throw new IOException("Unable to create snap directory "
+                        + this.snapDir);
+            }
         }
         txnLog = new FileTxnLog(this.dataDir);
         snapLog = new FileSnap(this.snapDir);

+ 17 - 2
src/java/main/org/apache/zookeeper/server/persistence/Util.java

@@ -24,6 +24,7 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.RandomAccessFile;
+import java.io.Serializable;
 import java.net.URI;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
@@ -170,7 +171,17 @@ public class Util {
         try {
             raf.seek(raf.length() - 5);
             byte bytes[] = new byte[5];
-            raf.read(bytes);
+            int readlen = 0;
+            int l;
+            while(readlen < 5 &&
+                  (l = raf.read(bytes, readlen, bytes.length - readlen)) >= 0) {
+                readlen += l;
+            }
+            if (readlen != bytes.length) {
+                LOG.info("Invalid snapshot " + f
+                        + " too short, len = " + readlen);
+                return false;
+            }
             ByteBuffer bb = ByteBuffer.wrap(bytes);
             int len = bb.getInt();
             byte b = bb.get();
@@ -271,7 +282,11 @@ public class Util {
      * Compare file file names of form "prefix.version". Sort order result
      * returned in order of version.
      */
-    private static class DataDirFileComparator implements Comparator<File> {
+    private static class DataDirFileComparator
+        implements Comparator<File>, Serializable
+    {
+        private static final long serialVersionUID = -2648639884525140318L;
+
         private String prefix;
         private boolean ascending;
         public DataDirFileComparator(String prefix, boolean ascending) {

+ 1 - 4
src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java

@@ -20,7 +20,6 @@
 package org.apache.zookeeper.server.quorum;
 
 import java.nio.ByteBuffer;
-import java.util.Collection;
 import java.util.Map;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -348,9 +347,7 @@ public class FastLeaderElection implements Election {
     }
 
     QuorumPeer self;
-    int port;
     volatile long logicalclock; /* Election instance */
-    Messenger messenger;
     long proposedLeader;
     long proposedZxid;
 
@@ -393,7 +390,7 @@ public class FastLeaderElection implements Election {
 
         sendqueue = new LinkedBlockingQueue<ToSend>();
         recvqueue = new LinkedBlockingQueue<Notification>();
-        messenger = new Messenger(manager);
+        new Messenger(manager);
     }
 
     private void leaveInstance() {

+ 5 - 1
src/java/main/org/apache/zookeeper/server/quorum/FollowerHandler.java

@@ -418,7 +418,11 @@ public class FollowerHandler extends Thread {
      * ping calls from the leader to the followers
      */
     public void ping() {
-        QuorumPacket ping = new QuorumPacket(Leader.PING, leader.lastProposed,
+        long id;
+        synchronized(leader) {
+            id = leader.lastProposed;
+        }
+        QuorumPacket ping = new QuorumPacket(Leader.PING, id,
                 null, null);
         queuePacket(ping);
     }

+ 2 - 1
src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java

@@ -64,8 +64,9 @@ public class LeaderZooKeeperServer extends ZooKeeperServer {
         commitProcessor = new CommitProcessor(toBeAppliedProcessor,
                 Long.toString(getServerId()), false);
         commitProcessor.start();
-        RequestProcessor proposalProcessor = new ProposalRequestProcessor(this,
+        ProposalRequestProcessor proposalProcessor = new ProposalRequestProcessor(this,
                 commitProcessor);
+        proposalProcessor.initialize();
         firstProcessor = new PrepRequestProcessor(this, proposalProcessor);
         ((PrepRequestProcessor)firstProcessor).start();
     }

+ 7 - 1
src/java/main/org/apache/zookeeper/server/quorum/ProposalRequestProcessor.java

@@ -39,9 +39,15 @@ public class ProposalRequestProcessor implements RequestProcessor {
         this.nextProcessor = nextProcessor;
         AckRequestProcessor ackProcessor = new AckRequestProcessor(zks.getLeader());
         syncProcessor = new SyncRequestProcessor(zks, ackProcessor);
+    }
+    
+    /**
+     * initialize this processor
+     */
+    public void initialize() {
         syncProcessor.start();
     }
-
+    
     public void processRequest(Request request) {
         // LOG.warn("Ack>>> cxid = " + request.cxid + " type = " +
         // request.type + " id = " + request.sessionId);

+ 7 - 12
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java

@@ -67,17 +67,7 @@ public class QuorumCnxManager {
      */
 
     static final int MAX_CONNECTION_ATTEMPTS = 2;
-
-    /*
-     * Packet size
-     */
-    int packetSize;
     
-    /*
-     * Challenge to initiate connections
-     */
-    long challenge;
-
     /*
      * Local IP address
      */
@@ -491,8 +481,13 @@ public class QuorumCnxManager {
                         senderWorkerMap.remove(sid);
                         ArrayBlockingQueue<ByteBuffer> bq = queueSendMap.get(sid);
                         if(bq != null){
-                            if (bq.size() == 0)
-                                bq.offer(b);
+                            if (bq.size() == 0) {
+                                boolean ret = bq.offer(b);
+                                if (!ret) {
+                                    // to appease findbugs
+                                    LOG.error("Not able to add to a quue of size 0");
+                                }
+                            }
                         } else LOG.error("No queue for server " + sid);
                     }
                 }

+ 5 - 1
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java

@@ -208,7 +208,11 @@ public class QuorumPeer extends Thread implements QuorumStats.Provider {
                         case LEADING:
                             responseBuffer.putLong(myid);
                             try {
-                                responseBuffer.putLong(leader.lastProposed);
+                                long proposed;
+                                synchronized(leader) {
+                                    proposed = leader.lastProposed;
+                                }
+                                responseBuffer.putLong(proposed);
                             } catch (NullPointerException npe) {
                                 // This can happen in state transitions,
                                 // just ignore the request

+ 43 - 0
src/java/test/config/findbugsExcludeFile.xml

@@ -65,4 +65,47 @@
     <Bug code="EI, EI2" />
   </Match>
 
+  <Match>
+    <Class name="org.apache.zookeeper.server.DataNode" />
+      <Bug code="EI2"/>
+  </Match>
+
+  <Match>
+    <Class name="org.apache.zookeeper.server.quorum.QuorumPacket" />
+       <Bug code="EI2, EI" />
+  </Match>
+
+  <Match>
+    <Class name="org.apache.zookeeper.ClientCnxn"/>
+      <Bug code="EI, EI2" />
+  </Match>
+
+  <Match>
+    <Class name="org.apache.zookeeper.server.DataNode"/>
+      <Field name="children"/> 
+      <Bug code="IS"/>
+  </Match>
+ <Match>
+   <Class name="org.apache.zookeeper.server.quorum.Leader"/>
+     <Field name="lastProposed"/>
+     <Bug code="IS"/>
+  </Match>
+  <Match>
+     <Class name="org.apache.zookeeper.server.quorum.FollowerSessionTracker"/>
+       <Bug code="UrF"/>
+  </Match>
+  <Match>
+    <Class name="org.apache.zookeeper.server.quorum.AuthFastLeaderElection$Messenger$WorkerSender"/>
+    <Method name="process"/>
+    <Bug code="RV"/>
+  </Match>
+
+  <!-- these are old classes just for upgrading and should go away -->
+  <Match>
+    <Class name="org.apache.zookeeper.server.upgrade.DataNodeV1"/>
+  </Match> 
+
+  <Match>
+    <Class name="org.apache.zookeeper.server.upgrade.DataTreeV1"/>
+  </Match>
 </FindBugsFilter>

+ 1 - 1
src/java/test/org/apache/zookeeper/server/CRCTest.java

@@ -119,7 +119,7 @@ public class CRCTest extends TestCase implements Watcher{
         File tmpDir = ClientBase.createTmpDir();
         ClientBase.setupTestEnv();
         zks = new ZooKeeperServer(tmpDir, tmpDir, 3000);
-        SyncRequestProcessor.snapCount = 150;
+        SyncRequestProcessor.setSnapCount(150);
         final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
         NIOServerCnxn.Factory f = new NIOServerCnxn.Factory(PORT);
         f.startup(zks);

+ 2 - 2
src/java/test/org/apache/zookeeper/server/InvalidSnapshotTest.java

@@ -52,7 +52,7 @@ public class InvalidSnapshotTest extends TestCase implements Watcher {
        File tmpDir = ClientBase.createTmpDir();
        ClientBase.setupTestEnv();
        zks = new ZooKeeperServer(tmpDir, tmpDir, 3000);
-       SyncRequestProcessor.snapCount = 100;
+       SyncRequestProcessor.setSnapCount(100);
        final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
        NIOServerCnxn.Factory f = new NIOServerCnxn.Factory(PORT);
        f.startup(zks);
@@ -73,7 +73,7 @@ public class InvalidSnapshotTest extends TestCase implements Watcher {
        raf.close();
        // now restart the server and see if it starts
        zks = new ZooKeeperServer(tmpDir, tmpDir, 3000);
-       SyncRequestProcessor.snapCount = 100;
+       SyncRequestProcessor.setSnapCount(100);
        f = new NIOServerCnxn.Factory(PORT);
        f.startup(zks);
        assertTrue("waiting for server being up ", 

+ 16 - 16
src/java/test/org/apache/zookeeper/test/ACLTest.java

@@ -45,7 +45,7 @@ public class ACLTest extends TestCase implements Watcher {
     private static String HOSTPORT = "127.0.0.1:2355";
     ZooKeeperServer zks;
     private CountDownLatch startSignal;
-    
+
     @Override
     protected void setUp() throws Exception {
         LOG.info("STARTING " + getName());
@@ -56,20 +56,20 @@ public class ACLTest extends TestCase implements Watcher {
     }
 
     /**
-     * Verify that acl optimization of storing just 
-     * a few acls and there references in the data 
+     * Verify that acl optimization of storing just
+     * a few acls and there references in the data
      * node is actually working.
      */
     public void testAcls() throws Exception {
         File tmpDir = ClientBase.createTmpDir();
         ClientBase.setupTestEnv();
         zks = new ZooKeeperServer(tmpDir, tmpDir, 3000);
-        SyncRequestProcessor.snapCount = 1000;
+        SyncRequestProcessor.setSnapCount(1000);
         final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
         NIOServerCnxn.Factory f = new NIOServerCnxn.Factory(PORT);
         f.startup(zks);
         LOG.info("starting up the zookeeper server .. waiting");
-        assertTrue("waiting for server being up", 
+        assertTrue("waiting for server being up",
                 ClientBase.waitForServerUp(HOSTPORT,CONNECTION_TIMEOUT));
         ZooKeeper zk = new ZooKeeper(HOSTPORT, CONNECTION_TIMEOUT, this);
         String path;
@@ -102,17 +102,17 @@ public class ACLTest extends TestCase implements Watcher {
 
         zks = new ZooKeeperServer(tmpDir, tmpDir, 3000);
         f = new NIOServerCnxn.Factory(PORT);
-        
+
         f.startup(zks);
 
         assertTrue("waiting for server up",
                    ClientBase.waitForServerUp(HOSTPORT,
                                        CONNECTION_TIMEOUT));
-        
+
         startSignal.await(CONNECTION_TIMEOUT,
                 TimeUnit.MILLISECONDS);
         assertTrue("count == 0", startSignal.getCount() == 0);
-        
+
         assertTrue("acl map ", (101 == zks.dataTree.longKeyMap.size()));
         for (int j =200; j < 205; j++) {
             path = "/" + j;
@@ -133,20 +133,20 @@ public class ACLTest extends TestCase implements Watcher {
         assertTrue("waiting for server down",
                    ClientBase.waitForServerDown(HOSTPORT,
                            ClientBase.CONNECTION_TIMEOUT));
-        
+
     }
-    
-    /*                  
-     * (non-Javadoc)    
-     *                          
+
+    /*
+     * (non-Javadoc)
+     *
      * @see org.apache.zookeeper.Watcher#process(org.apache.zookeeper.WatcherEvent)
-     */         
+     */
     public void process(WatchedEvent event) {
         LOG.info("Event:" + event.getState() + " " + event.getType() + " " + event.getPath());
         if (event.getState() == KeeperState.SyncConnected
                 && startSignal != null && startSignal.getCount() > 0)
-        {              
-            startSignal.countDown();      
+        {
+            startSignal.countDown();
         }
     }
 }

+ 9 - 9
src/java/test/org/apache/zookeeper/test/AsyncOpsTest.java

@@ -35,19 +35,19 @@ import org.junit.Test;
 /**
  * Functional testing of asynchronous operations, both positive and negative
  * testing.
- * 
+ *
  * This just scratches the surface, but exercises the basic async functionality.
  */
 public class AsyncOpsTest extends ClientBase {
     private static final Logger LOG = Logger.getLogger(AsyncOpsTest.class);
 
     private ZooKeeper zk;
-    
+
     @Before
     @Override
     protected void setUp() throws Exception {
         super.setUp();
-        
+
         LOG.info("STARTING " + getName());
 
         zk = createClient();
@@ -58,9 +58,9 @@ public class AsyncOpsTest extends ClientBase {
     @Override
     protected void tearDown() throws Exception {
         zk.close();
-        
+
         super.tearDown();
-        
+
         LOG.info("Test clients shutting down");
 
         LOG.info("FINISHED " + getName());
@@ -74,23 +74,23 @@ public class AsyncOpsTest extends ClientBase {
     @Test
     public void testAsyncCreateThree() {
         CountDownLatch latch = new CountDownLatch(3);
-        
+
         StringCB op1 = new StringCB(zk, latch);
         op1.setPath("/op1");
         StringCB op2 = new StringCB(zk, latch);
         op2.setPath("/op2");
         StringCB op3 = new StringCB(zk, latch);
         op3.setPath("/op3");
-        
+
         op1.create();
         op2.create();
         op3.create();
-        
+
         op1.verify();
         op2.verify();
         op3.verify();
     }
-    
+
     @Test
     public void testAsyncCreateFailure_NodeExists() {
         new StringCB(zk).verifyCreateFailure_NodeExists();

+ 15 - 16
src/java/test/org/apache/zookeeper/test/ClientBase.java

@@ -77,7 +77,7 @@ public abstract class ClientBase extends TestCase {
         // XXX this doesn't need to be volatile! (Should probably be final)
         volatile CountDownLatch clientConnected;
         volatile boolean connected;
-        
+
         public CountdownWatcher() {
             reset();
         }
@@ -107,7 +107,7 @@ public abstract class ClientBase extends TestCase {
             }
             if (!connected) {
                 throw new TimeoutException("Did not connect");
-         
+
             }
         }
         synchronized void waitForDisconnected(long timeout) throws InterruptedException, TimeoutException {
@@ -119,11 +119,11 @@ public abstract class ClientBase extends TestCase {
             }
             if (connected) {
                 throw new TimeoutException("Did not disconnect");
-         
+
             }
         }
     }
-    
+
     protected ZooKeeper createClient()
         throws IOException, InterruptedException
     {
@@ -226,7 +226,7 @@ public abstract class ClientBase extends TestCase {
         }
         return false;
     }
-    
+
     static void verifyThreadTerminated(Thread thread, long millis)
         throws InterruptedException
     {
@@ -234,7 +234,7 @@ public abstract class ClientBase extends TestCase {
         if (thread.isAlive()) {
             LOG.error("Thread " + thread.getName() + " : "
                     + Arrays.toString(thread.getStackTrace()));
-            assertFalse("thread " + thread.getName() 
+            assertFalse("thread " + thread.getName()
                     + " still alive after join", true);
         }
     }
@@ -272,7 +272,7 @@ public abstract class ClientBase extends TestCase {
 
         return factory;
     }
-    
+
     static void shutdownServerInstance(NIOServerCnxn.Factory factory,
             String hostPort)
     {
@@ -285,7 +285,7 @@ public abstract class ClientBase extends TestCase {
                                                     CONNECTION_TIMEOUT));
         }
     }
-    
+
     /**
      * Test specific setup
      */
@@ -295,21 +295,20 @@ public abstract class ClientBase extends TestCase {
         // resulting in test failure (client timeout on first session).
         // set env and directly in order to handle static init/gc issues
         System.setProperty("zookeeper.preAllocSize", "100");
-        FileTxnLog.setPreallocSize(100);
+        FileTxnLog.setPreallocSize(100 * 1024);
     }
-    
+
     @Override
     protected void setUp() throws Exception {
         LOG.info("STARTING " + getName());
+        setupTestEnv();
 
         JMXEnv.setUp();
-        
+
         tmpDir = createTmpDir(BASETEST);
-        
-        setupTestEnv();
 
         startServer();
-        
+
         LOG.info("Client test setup finished");
     }
 
@@ -319,7 +318,7 @@ public abstract class ClientBase extends TestCase {
         // ensure that only server and data bean are registered
         JMXEnv.ensureOnly("InMemoryDataTree", "StandaloneServer_port");
     }
-    
+
     protected void stopServer() throws Exception {
         LOG.info("STOPPING server");
         shutdownServerInstance(serverFactory, hostPort);
@@ -348,7 +347,7 @@ public abstract class ClientBase extends TestCase {
     public static MBeanServerConnection jmxConn() throws IOException {
         return JMXEnv.conn();
     }
-    
+
     private static boolean recursiveDelete(File d) {
         if (d.isDirectory()) {
             File children[] = d.listFiles();

+ 2 - 2
src/java/test/org/apache/zookeeper/test/ClientTest.java

@@ -532,14 +532,14 @@ public class ClientTest extends ClientBase {
 
         
         //check for the code path that throws at server
-        PrepRequestProcessor.failCreate = true;
+        PrepRequestProcessor.setFailCreate(true);
         try {
             zk.create("/m", null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
             assertTrue(false);
         } catch(KeeperException.BadArgumentsException be) {
             // catch this.
         }
-        PrepRequestProcessor.failCreate = false;
+        PrepRequestProcessor.setFailCreate(false);
         zk.create("/.foo", null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
         zk.create("/.f.", null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
         zk.create("/..f", null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);

+ 62 - 63
src/java/test/org/apache/zookeeper/test/FLENewEpochTest.java

@@ -45,10 +45,10 @@ public class FLENewEpochTest extends TestCase {
     File tmpdir[];
     int port[];
     volatile int [] round;
-    
+
     Semaphore start0;
     Semaphore finish3, finish0;
-    
+
     @Override
     public void setUp() throws Exception {
         count = 3;
@@ -64,11 +64,11 @@ public class FLENewEpochTest extends TestCase {
         round[0] = 0;
         round[1] = 0;
         round[2] = 0;
-        
+
         start0 = new Semaphore(0);
         finish0 = new Semaphore(0);
         finish3 = new Semaphore(0);
-        
+
         LOG.info("SetUp " + getName());
     }
 
@@ -82,7 +82,6 @@ public class FLENewEpochTest extends TestCase {
 
 
     class LEThread extends Thread {
-        FastLeaderElection le;
         int i;
         QuorumPeer peer;
 
@@ -90,67 +89,67 @@ public class FLENewEpochTest extends TestCase {
             this.i = i;
             this.peer = peer;
             LOG.info("Constructor: " + getName());
-            
+
         }
 
         public void run(){
-        	boolean flag = true;
+            boolean flag = true;
             try{
-            	while(flag){
-            		Vote v = null;
-            		peer.setPeerState(ServerState.LOOKING);
-            		LOG.info("Going to call leader election again: " + i);
-            		v = peer.getElectionAlg().lookForLeader();
-
-            		if(v == null){
-            			fail("Thread " + i + " got a null vote");
-            		}
-
-            		/*
-            		 * A real zookeeper would take care of setting the current vote. Here
-            		 * we do it manually.
-            		 */
-            		peer.setCurrentVote(v);
-
-            		LOG.info("Finished election: " + i + ", " + v.id);
-            		//votes[i] = v;
-
-            		switch(i){
-            		case 0:
-            			LOG.info("First peer, do nothing, just join");
-            			if(finish0.tryAcquire(1000, java.util.concurrent.TimeUnit.MILLISECONDS)){
-            			//if(threads.get(0).peer.getPeerState() == ServerState.LEADING ){
-            			    LOG.info("Setting flag to false");
-            			    flag = false;
-            			}
-            			break;
-            		case 1:
-            			LOG.info("Second entering case");
-            			if(round[1] != 0){
-            			    finish0.release();
-            			    flag = false;
-            			}
-            			else{
-            				finish3.acquire();
-            				start0.release();
-            			}
-            			LOG.info("Second is going to start second round");
-            			round[1]++;
-            			break;
-            		case 2:
-            			LOG.info("Third peer, shutting it down");
-            			((FastLeaderElection) peer.getElectionAlg()).shutdown();
-            			peer.shutdown();
-            			flag = false;
-            			round[2] = 1;
-            			finish3.release();
-            			LOG.info("Third leaving");
-            			break;
-            		}
-            	}
+                while(flag){
+                    Vote v = null;
+                    peer.setPeerState(ServerState.LOOKING);
+                    LOG.info("Going to call leader election again: " + i);
+                    v = peer.getElectionAlg().lookForLeader();
+
+                    if(v == null){
+                        fail("Thread " + i + " got a null vote");
+                    }
+
+                    /*
+                     * A real zookeeper would take care of setting the current vote. Here
+                     * we do it manually.
+                     */
+                    peer.setCurrentVote(v);
+
+                    LOG.info("Finished election: " + i + ", " + v.id);
+                    //votes[i] = v;
+
+                    switch(i){
+                    case 0:
+                        LOG.info("First peer, do nothing, just join");
+                        if(finish0.tryAcquire(1000, java.util.concurrent.TimeUnit.MILLISECONDS)){
+                        //if(threads.get(0).peer.getPeerState() == ServerState.LEADING ){
+                            LOG.info("Setting flag to false");
+                            flag = false;
+                        }
+                        break;
+                    case 1:
+                        LOG.info("Second entering case");
+                        if(round[1] != 0){
+                            finish0.release();
+                            flag = false;
+                        }
+                        else{
+                            finish3.acquire();
+                            start0.release();
+                        }
+                        LOG.info("Second is going to start second round");
+                        round[1]++;
+                        break;
+                    case 2:
+                        LOG.info("Third peer, shutting it down");
+                        ((FastLeaderElection) peer.getElectionAlg()).shutdown();
+                        peer.shutdown();
+                        flag = false;
+                        round[2] = 1;
+                        finish3.release();
+                        LOG.info("Third leaving");
+                        break;
+                    }
+                }
             } catch (Exception e){
-            	e.printStackTrace();
-            }    
+                e.printStackTrace();
+            }
         }
     }
 
@@ -164,7 +163,7 @@ public class FLENewEpochTest extends TestCase {
           for(int i = 0; i < count; i++) {
               peers.put(Long.valueOf(i), new QuorumServer(i, new InetSocketAddress(baseport+100+i),
                       new InetSocketAddress(baseLEport+100+i)));
-              tmpdir[i] = File.createTempFile("letest", "test");
+              tmpdir[i] = ClientBase.createTmpDir();
               port[i] = baseport+i;
           }
 
@@ -183,7 +182,7 @@ public class FLENewEpochTest extends TestCase {
           LEThread thread = new LEThread(peer, 0);
           thread.start();
           threads.add(thread);
-          
+
           LOG.info("Started threads " + getName());
 
           for(int i = 0; i < threads.size(); i++) {

+ 55 - 58
src/java/test/org/apache/zookeeper/test/FLETest.java

@@ -38,48 +38,46 @@ import org.junit.Test;
 public class FLETest extends TestCase {
     protected static final Logger LOG = Logger.getLogger(FLETest.class);
 
-    class TestVote{
-	TestVote(int id, long leader){
-		this.leader = leader;
-		this.id = id;
-	}
-
-	long leader;
-	int id;
+    static class TestVote {
+        TestVote(int id, long leader) {
+            this.leader = leader;
+        }
+
+        long leader;
     }
- 
-    int countVotes(HashSet<TestVote> hs, long id){
-	int counter = 0;
-	for(TestVote v : hs){
-	   if(v.leader == id) counter++;
+
+    int countVotes(HashSet<TestVote> hs, long id) {
+        int counter = 0;
+        for(TestVote v : hs){
+            if(v.leader == id) counter++;
         }
 
-	return counter;
+        return counter;
     }
 
     int count;
     int baseport;
     int baseLEport;
-    HashMap<Long,QuorumServer> peers; 
+    HashMap<Long,QuorumServer> peers;
     ArrayList<LEThread> threads;
     HashMap<Integer, HashSet<TestVote> > voteMap;
     File tmpdir[];
     int port[];
     int successCount;
     Object finalObj;
-    
+
     volatile Vote votes[];
     volatile boolean leaderDies;
     volatile long leader = -1;
-    //volatile int round = 1; 
+    //volatile int round = 1;
     Random rand = new Random();
-    
+
     @Override
     public void setUp() throws Exception {
         count = 7;
         baseport= 33003;
         baseLEport = 43003;
-        
+
         peers = new HashMap<Long,QuorumServer>(count);
         threads = new ArrayList<LEThread>(count);
         voteMap = new HashMap<Integer, HashSet<TestVote> >();
@@ -88,7 +86,7 @@ public class FLETest extends TestCase {
         port = new int[count];
         successCount = 0;
         finalObj = new Object();
-        
+
         LOG.info("SetUp " + getName());
     }
 
@@ -99,9 +97,8 @@ public class FLETest extends TestCase {
         }
         LOG.info("FINISHED " + getName());
     }
-    
+
     class LEThread extends Thread {
-        FastLeaderElection le;
         int i;
         QuorumPeer peer;
         //int peerRound = 1;
@@ -118,28 +115,28 @@ public class FLETest extends TestCase {
                     peer.setPeerState(ServerState.LOOKING);
                     LOG.info("Going to call leader election again.");
                     v = peer.getElectionAlg().lookForLeader();
-                    if(v == null){ 
+                    if(v == null){
                         LOG.info("Thread " + i + " got a null vote");
                         break;
                     }
-                    
+
                     /*
                      * A real zookeeper would take care of setting the current vote. Here
                      * we do it manually.
                      */
                     peer.setCurrentVote(v);
-            
+
                     LOG.info("Finished election: " + i + ", " + v.id);
                     votes[i] = v;
-                    
+
                     /*
                      * Get the current value of the logical clock for this peer.
                      */
                     int lc = (int) ((FastLeaderElection) peer.getElectionAlg()).getLogicalClock();
-                    
+
                     if (v.id == ((long) i)) {
                         /*
-                         * A leader executes this part of the code. If it is the first leader to be 
+                         * A leader executes this part of the code. If it is the first leader to be
                          * elected, then it fails right after. Otherwise, it waits until it has enough
                          * followers supporting it.
                          */
@@ -151,22 +148,22 @@ public class FLETest extends TestCase {
                                 ((FastLeaderElection) peer.getElectionAlg()).shutdown();
                                 leader = -1;
                                 LOG.info("Leader " + i + " dead");
-                                
-                                //round++; 
+
+                                //round++;
                                 FLETest.this.notifyAll();
-                                
+
                                 break;
-                                
+
                             } else {
                                 synchronized(voteMap){
                                     if(voteMap.get(lc) == null)
                                         voteMap.put(lc, new HashSet<TestVote>());
                                     HashSet<TestVote> hs = voteMap.get(lc);
                                     hs.add(new TestVote(i, v.id));
-                                    
+
                                     if(countVotes(hs, v.id) > (count/2)){
                                         leader = i;
-                                        LOG.info("Got majority: " + i);   
+                                        LOG.info("Got majority: " + i);
                                     } else {
                                         voteMap.wait(3000);
                                         LOG.info("Notified or expired: " + i);
@@ -175,7 +172,7 @@ public class FLETest extends TestCase {
                                             leader = i;
                                             LOG.info("Got majority: " + i);
                                         } else {
-                                            //round++; 
+                                            //round++;
                                         }
                                     }
                                 }
@@ -186,45 +183,45 @@ public class FLETest extends TestCase {
                                         successCount++;
                                         if(successCount > (count/2)) finalObj.notify();
                                     }
-                                    
+
                                     break;
                                 }
                             }
                         }
                     } else {
                         /*
-                         * Followers execute this part. They first add their vote to voteMap, and then 
+                         * Followers execute this part. They first add their vote to voteMap, and then
                          * they wait for bounded amount of time. A leader notifies followers through the
                          * FLETest.this object.
-                         * 
+                         *
                          * Note that I can get FLETest.this, and then voteMap before adding the vote of
                          * a follower, otherwise a follower would be blocked out until the leader notifies
                          * or leaves the synchronized block on FLEtest.this.
                          */
-                        
-                        
+
+
                         LOG.info("Logical clock " + ((FastLeaderElection) peer.getElectionAlg()).getLogicalClock());
                         synchronized(voteMap){
                             LOG.info("Voting on " + votes[i].id + ", round " + ((FastLeaderElection) peer.getElectionAlg()).getLogicalClock());
                             if(voteMap.get(lc) == null)
                                 voteMap.put(lc, new HashSet<TestVote>());
-                            HashSet<TestVote> hs = voteMap.get(lc);    
-                            hs.add(new TestVote(i, votes[i].id)); 
+                            HashSet<TestVote> hs = voteMap.get(lc);
+                            hs.add(new TestVote(i, votes[i].id));
                             if(countVotes(hs, votes[i].id) > (count/2)){
                                 LOG.info("Logical clock: " + lc + ", " + votes[i].id);
                                 voteMap.notify();
-                            }    
+                            }
                         }
-                        
+
                         /*
                          * In this part a follower waits until the leader notifies it, and remove its
                          * vote if the leader takes too long to respond.
                          */
                         synchronized(FLETest.this){
                             if (leader != votes[i].id) FLETest.this.wait(3000);
-                        
+
                             LOG.info("The leader: " + leader + " and my vote " + votes[i].id);
-                            synchronized(voteMap){ 
+                            synchronized(voteMap){
                                 if (leader == votes[i].id) {
                                     synchronized(finalObj){
                                         successCount++;
@@ -257,22 +254,22 @@ public class FLETest extends TestCase {
             }
         }
     }
-    
+
     @Test
     public void testLE() throws Exception {
-       
+
         FastLeaderElection le[] = new FastLeaderElection[count];
         leaderDies = true;
         boolean allowOneBadLeader = leaderDies;
-       
+
         LOG.info("TestLE: " + getName()+ ", " + count);
         for(int i = 0; i < count; i++) {
-            peers.put(Long.valueOf(i), new QuorumServer(i, new InetSocketAddress(baseport+100+i), 
+            peers.put(Long.valueOf(i), new QuorumServer(i, new InetSocketAddress(baseport+100+i),
                     new InetSocketAddress(baseLEport+100+i)));
-            tmpdir[i] = File.createTempFile("letest", "test");
-            port[i] = baseport+i;    
+            tmpdir[i] = ClientBase.createTmpDir();
+            port[i] = baseport+i;
         }
-        
+
         for(int i = 0; i < le.length; i++) {
             QuorumPeer peer = new QuorumPeer(peers, tmpdir[i], tmpdir[i], port[i], 3, i, 2, 2, 2);
             peer.startLeaderElection();
@@ -281,8 +278,8 @@ public class FLETest extends TestCase {
             threads.add(thread);
         }
         LOG.info("Started threads " + getName());
-        
-        
+
+
         int waitCounter = 0;
         synchronized(finalObj){
             while((successCount <= count/2) && (waitCounter < 50)){
@@ -290,7 +287,7 @@ public class FLETest extends TestCase {
                 waitCounter++;
             }
         }
-        
+
        /*
         * Lists what threads haven-t joined. A thread doesn't join if it hasn't decided
         * upon a leader yet. It can happen that a peer is slow or disconnected, and it can
@@ -301,14 +298,14 @@ public class FLETest extends TestCase {
                 LOG.info("Threads didn't join: " + i);
             }
         }
-       
+
        /*
         * If we have a majority, then we are good to go.
         */
        if(successCount <= count/2){
            fail("Fewer than a a majority has joined");
        }
-       
+
        if(threads.get((int) leader).isAlive()){
            fail("Leader hasn't joined: " + leader);
        }

+ 32 - 46
src/java/test/org/apache/zookeeper/test/HierarchicalQuorumTest.java

@@ -17,77 +17,64 @@
  */
 
 package org.apache.zookeeper.test;
-import java.io.IOException;
-import java.util.Properties;
-
-import java.io.File;
 import java.io.ByteArrayInputStream;
+import java.io.File;
 import java.net.InetSocketAddress;
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.Properties;
 import java.util.Random;
 
-import org.apache.zookeeper.KeeperException;
+import junit.framework.TestCase;
+
+import org.apache.log4j.Logger;
 import org.apache.zookeeper.server.quorum.FastLeaderElection;
 import org.apache.zookeeper.server.quorum.QuorumPeer;
 import org.apache.zookeeper.server.quorum.Vote;
 import org.apache.zookeeper.server.quorum.QuorumPeer.QuorumServer;
 import org.apache.zookeeper.server.quorum.QuorumPeer.ServerState;
 import org.apache.zookeeper.server.quorum.flexible.QuorumHierarchical;
-
-import org.apache.log4j.Logger;
-import junit.framework.TestCase;
 import org.junit.Before;
 import org.junit.Test;
 
 public class HierarchicalQuorumTest extends TestCase {
     private static final Logger LOG = Logger.getLogger(HierarchicalQuorumTest.class);
-    
-    class TestVote{
-        TestVote(int id, long leader){
-            this.leader = leader;
-            this.id = id;
-        }
-        long leader;
-        int id;
-    }
-    
+
     Properties qp;
-    
+
     int count;
     int baseport;
     int baseLEport;
-    HashMap<Long,QuorumServer> peers; 
+    HashMap<Long,QuorumServer> peers;
     ArrayList<LEThread> threads;
     File tmpdir[];
     int port[];
     Object finalObj;
-    
+
     volatile Vote votes[];
     volatile boolean leaderDies;
     volatile long leader = -1;
     Random rand = new Random();
-    
-    
+
+
     @Before
     @Override
     protected void setUp() throws Exception {
         count = 9;
         baseport= 33003;
         baseLEport = 43003;
-        
+
         peers = new HashMap<Long,QuorumServer>(count);
         threads = new ArrayList<LEThread>(count);
         votes = new Vote[count];
         tmpdir = new File[count];
         port = new int[count];
         finalObj = new Object();
-        
+
         String config = "group.1=0:1:2\n" +
         "group.2=3:4:5\n" +
-        "group.3=6:7:8\n\n" + 
-        "weight.0=1\n" + 
+        "group.3=6:7:8\n\n" +
+        "weight.0=1\n" +
         "weight.1=1\n" +
         "weight.2=1\n" +
         "weight.3=1\n" +
@@ -96,23 +83,22 @@ public class HierarchicalQuorumTest extends TestCase {
         "weight.6=1\n" +
         "weight.7=1\n" +
         "weight.8=1";
-        
+
         ByteArrayInputStream is = new ByteArrayInputStream(config.getBytes());
-        this.qp = new Properties(); 
+        this.qp = new Properties();
         qp.load(is);
-        
+
         LOG.info("SetUp " + getName());
     }
-    
+
     protected void tearDown() throws Exception {
         for(int i = 0; i < threads.size(); i++) {
             ((FastLeaderElection) threads.get(i).peer.getElectionAlg()).shutdown();
         }
         LOG.info("FINISHED " + getName());
     }
-    
+
     class LEThread extends Thread {
-        FastLeaderElection le;
         int i;
         QuorumPeer peer;
         //int peerRound = 1;
@@ -127,25 +113,25 @@ public class HierarchicalQuorumTest extends TestCase {
             try {
                 Vote v = null;
                 while(true){
-                    
+
                     //while(true) {
                     peer.setPeerState(ServerState.LOOKING);
                     LOG.info("Going to call leader election.");
                     v = peer.getElectionAlg().lookForLeader();
-                    if(v == null){ 
+                    if(v == null){
                         LOG.info("Thread " + i + " got a null vote");
                         return;
                     }
-                    
+
                     /*
                      * A real zookeeper would take care of setting the current vote. Here
                      * we do it manually.
                      */
                     peer.setCurrentVote(v);
-            
+
                     LOG.info("Finished election: " + i + ", " + v.id);
                     votes[i] = v;
-                
+
                     if((peer.getPeerState() == ServerState.FOLLOWING) ||
                             (peer.getPeerState() == ServerState.LEADING)) break;
                 }
@@ -155,19 +141,19 @@ public class HierarchicalQuorumTest extends TestCase {
             }
         }
     }
-    
+
     @Test
     public void testHierarchicalQuorum() throws Exception {
         FastLeaderElection le[] = new FastLeaderElection[count];
-       
+
         LOG.info("TestHierarchicalQuorum: " + getName()+ ", " + count);
         for(int i = 0; i < count; i++) {
-            peers.put(Long.valueOf(i), new QuorumServer(i, new InetSocketAddress(baseport+100+i), 
+            peers.put(Long.valueOf(i), new QuorumServer(i, new InetSocketAddress(baseport+100+i),
                     new InetSocketAddress(baseLEport+100+i)));
-            tmpdir[i] = File.createTempFile("letest", "test");
-            port[i] = baseport+i;    
+            tmpdir[i] = ClientBase.createTmpDir();
+            port[i] = baseport+i;
         }
-        
+
         for(int i = 0; i < le.length; i++) {
             QuorumHierarchical hq = new QuorumHierarchical(qp);
             QuorumPeer peer = new QuorumPeer(peers, tmpdir[i], tmpdir[i], port[i], 3, i, 2, 2, 2, hq);
@@ -177,7 +163,7 @@ public class HierarchicalQuorumTest extends TestCase {
             threads.add(thread);
         }
         LOG.info("Started threads " + getName());
-        
+
         for(int i = 0; i < threads.size(); i++) {
             threads.get(i).join(15000);
             if (threads.get(i).isAlive()) {

+ 1 - 1
src/java/test/org/apache/zookeeper/test/InvalidSnapshotTest.java

@@ -58,7 +58,7 @@ public class InvalidSnapshotTest extends TestCase implements Watcher {
     public void testSnapshot() throws Exception {
         File snapDir = new File(testData, "invalidsnap");
         zks = new ZooKeeperServer(snapDir, snapDir, 3000);
-        SyncRequestProcessor.snapCount = 1000;
+        SyncRequestProcessor.setSnapCount(1000);
         final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
         NIOServerCnxn.Factory f = new NIOServerCnxn.Factory(PORT);
         f.startup(zks);

+ 4 - 4
src/java/test/org/apache/zookeeper/test/LETest.java

@@ -34,7 +34,7 @@ import org.apache.zookeeper.server.quorum.QuorumPeer.QuorumServer;
 public class LETest extends TestCase {
     volatile Vote votes[];
     volatile boolean leaderDies;
-    volatile long leader = -1; 
+    volatile long leader = -1;
     Random rand = new Random();
     class LEThread extends Thread {
         LeaderElection le;
@@ -59,7 +59,7 @@ public class LETest extends TestCase {
                                 System.out.println("Leader " + i + " dying");
                                 leader = -2;
                             } else {
-                                leader = i; 
+                                leader = i;
                             }
                             LETest.this.notifyAll();
                         }
@@ -92,8 +92,8 @@ public class LETest extends TestCase {
         votes = new Vote[count];
         for(int i = 0; i < count; i++) {
             peers.put(Long.valueOf(i), new QuorumServer(i, new InetSocketAddress("127.0.0.1", baseport+100+i)));
-            tmpdir[i] = File.createTempFile("letest", "test");
-            port[i] = baseport+i;    
+            tmpdir[i] = ClientBase.createTmpDir();
+            port[i] = baseport+i;
         }
         LeaderElection le[] = new LeaderElection[count];
         leaderDies = true;

+ 1 - 1
src/java/test/org/apache/zookeeper/test/PurgeTxnTest.java

@@ -52,7 +52,7 @@ public class PurgeTxnTest extends TestCase implements  Watcher {
         File tmpDir = ClientBase.createTmpDir();
         ClientBase.setupTestEnv();
         zks = new ZooKeeperServer(tmpDir, tmpDir, 3000);
-        SyncRequestProcessor.snapCount = 100;
+        SyncRequestProcessor.setSnapCount(100);
         final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
         NIOServerCnxn.Factory f = new NIOServerCnxn.Factory(PORT);
         f.startup(zks);

+ 5 - 8
src/java/test/org/apache/zookeeper/test/QuorumBase.java

@@ -34,18 +34,15 @@ import org.junit.After;
 public class QuorumBase extends ClientBase {
     private static final Logger LOG = Logger.getLogger(QuorumBase.class);
 
-
-
     File s1dir, s2dir, s3dir, s4dir, s5dir;
     QuorumPeer s1, s2, s3, s4, s5;
 
     protected void setUp() throws Exception {
         LOG.info("STARTING " + getName());
+        setupTestEnv();
 
         JMXEnv.setUp();
 
-        setupTestEnv();
-
         hostPort = "127.0.0.1:2181,127.0.0.1:2182,127.0.0.1:2183,127.0.0.1:2184,127.0.0.1:2185";
 
         s1dir = ClientBase.createTmpDir();
@@ -108,11 +105,11 @@ public class QuorumBase extends ClientBase {
         // make sure we have these 5 servers listed
         Set<String> ensureNames = new LinkedHashSet<String>();
         for (int i = 1; i <= 5; i++) {
-            ensureNames.add("InMemoryDataTree"); 
+            ensureNames.add("InMemoryDataTree");
         }
         for (int i = 1; i <= 5; i++) {
             ensureNames.add("name0=ReplicatedServer_id" + i
-                 + ",name1=replica." + i + ",name2="); 
+                 + ",name1=replica." + i + ",name2=");
         }
         for (int i = 1; i <= 5; i++) {
             for (int j = 1; j <= 5; j++) {
@@ -121,7 +118,7 @@ public class QuorumBase extends ClientBase {
             }
         }
         for (int i = 1; i <= 5; i++) {
-            ensureNames.add("name0=ReplicatedServer_id" + i); 
+            ensureNames.add("name0=ReplicatedServer_id" + i);
         }
         JMXEnv.ensureAll(ensureNames.toArray(new String[ensureNames.size()]));
     }
@@ -144,7 +141,7 @@ public class QuorumBase extends ClientBase {
         }
 
         JMXEnv.tearDown();
-        
+
         LOG.info("FINISHED " + getName());
     }
 

+ 3 - 3
src/java/test/org/apache/zookeeper/test/RecoveryTest.java

@@ -74,8 +74,8 @@ public class RecoveryTest extends TestCase implements Watcher {
         ClientBase.setupTestEnv();
         ZooKeeperServer zs = new ZooKeeperServer(tmpDir, tmpDir, 3000);
 
-        int oldSnapCount = SyncRequestProcessor.snapCount;
-        SyncRequestProcessor.snapCount = 1000;
+        int oldSnapCount = SyncRequestProcessor.getSnapCount();
+        SyncRequestProcessor.setSnapCount(1000);
         try {
             final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
             NIOServerCnxn.Factory f = new NIOServerCnxn.Factory(PORT);
@@ -190,7 +190,7 @@ public class RecoveryTest extends TestCase implements Watcher {
                        ClientBase.waitForServerDown(HOSTPORT,
                                                     CONNECTION_TIMEOUT));
         } finally {
-            SyncRequestProcessor.snapCount = oldSnapCount;
+            SyncRequestProcessor.setSnapCount(oldSnapCount);
         }
     }
 

+ 1 - 1
src/java/test/org/apache/zookeeper/test/UpgradeTest.java

@@ -66,7 +66,7 @@ public class UpgradeTest extends TestCase implements Watcher {
         UpgradeMain upgrade = new UpgradeMain(upgradeDir, upgradeDir);
         upgrade.runUpgrade();
         zks = new ZooKeeperServer(upgradeDir, upgradeDir, 3000);
-        SyncRequestProcessor.snapCount = 1000;
+        SyncRequestProcessor.setSnapCount(1000);
         final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
         NIOServerCnxn.Factory f = new NIOServerCnxn.Factory(PORT);
         f.startup(zks);

+ 10 - 10
src/java/test/org/apache/zookeeper/test/WatcherTest.java

@@ -198,13 +198,13 @@ public class WatcherTest extends ClientBase {
 
     @Test
     public void testWatcherAutoResetDisabledWithGlobal() throws Exception {
-        ClientCnxn.disableAutoWatchReset = true;
+        ClientCnxn.setDisableAutoResetWatch(true);
         testWatcherAutoResetWithGlobal();
     }
 
     @Test
     public void testWatcherAutoResetDisabledWithLocal() throws Exception {
-        ClientCnxn.disableAutoWatchReset = true;
+        ClientCnxn.setDisableAutoResetWatch(true);
         testWatcherAutoResetWithLocal();
     }
 
@@ -231,7 +231,7 @@ public class WatcherTest extends ClientBase {
         localWatcher.waitForDisconnected(500);
         startServer();
         globalWatcher.waitForConnected(3000);
-        if (!isGlobal && !ClientCnxn.disableAutoWatchReset) {
+        if (!isGlobal && !ClientCnxn.getDisableAutoResetWatch()) {
             localWatcher.waitForConnected(500);
         }
 
@@ -241,7 +241,7 @@ public class WatcherTest extends ClientBase {
                 CreateMode.PERSISTENT);
 
         WatchedEvent e = localWatcher.events.poll(10, TimeUnit.SECONDS);
-        if (!ClientCnxn.disableAutoWatchReset) {
+        if (!ClientCnxn.getDisableAutoResetWatch()) {
             assertEquals(e.getPath(), EventType.NodeDataChanged, e.getType());
             assertEquals("/watchtest/child", e.getPath());
         } else {
@@ -249,7 +249,7 @@ public class WatcherTest extends ClientBase {
         }
 
         e = localWatcher.events.poll(10, TimeUnit.SECONDS);
-        if (!ClientCnxn.disableAutoWatchReset) {
+        if (!ClientCnxn.getDisableAutoResetWatch()) {
             // The create will trigger the get children and the exist
             // watches
             assertEquals(EventType.NodeCreated, e.getType());
@@ -259,7 +259,7 @@ public class WatcherTest extends ClientBase {
         }
 
         e = localWatcher.events.poll(10, TimeUnit.SECONDS);
-        if (!ClientCnxn.disableAutoWatchReset) {
+        if (!ClientCnxn.getDisableAutoResetWatch()) {
             assertEquals(EventType.NodeChildrenChanged, e.getType());
             assertEquals("/watchtest", e.getPath());
         } else {
@@ -275,11 +275,11 @@ public class WatcherTest extends ClientBase {
         try {
         try {
             localWatcher.waitForDisconnected(500);
-            if (!isGlobal && !ClientCnxn.disableAutoWatchReset) {
+            if (!isGlobal && !ClientCnxn.getDisableAutoResetWatch()) {
                 fail("Got an event when I shouldn't have");
             }
         } catch(TimeoutException toe) {
-            if (ClientCnxn.disableAutoWatchReset) {
+            if (ClientCnxn.getDisableAutoResetWatch()) {
                 fail("Didn't get an event when I should have");
             }
             // Else what we are expecting since there are no outstanding watches
@@ -320,7 +320,7 @@ public class WatcherTest extends ClientBase {
         localWatcher.waitForDisconnected(500);
         startServer();
         globalWatcher.waitForConnected(3000);
-        if (!isGlobal && !ClientCnxn.disableAutoWatchReset) {
+        if (!isGlobal && !ClientCnxn.getDisableAutoResetWatch()) {
             localWatcher.waitForConnected(500);
         }
 
@@ -328,7 +328,7 @@ public class WatcherTest extends ClientBase {
         zk.delete("/watchtest", -1);
 
         e = localWatcher.events.poll(10, TimeUnit.SECONDS);
-        if (!ClientCnxn.disableAutoWatchReset) {
+        if (!ClientCnxn.getDisableAutoResetWatch()) {
             assertEquals(EventType.NodeDeleted, e.getType());
             assertEquals("/watchtest/child", e.getPath());
         } else {