You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by el...@apache.org on 2014/01/31 19:54:14 UTC

[5/5] git commit: UTF8, unnecessary object creation (String, Boolean)

UTF8, unnecessary object creation (String, Boolean)


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/3bc08e9c
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/3bc08e9c
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/3bc08e9c

Branch: refs/heads/2292-findbugs
Commit: 3bc08e9cf0923ea3be5da49f13e8a6c1f19157e4
Parents: e8be283
Author: Josh Elser <el...@apache.org>
Authored: Fri Jan 31 13:52:56 2014 -0500
Committer: Josh Elser <el...@apache.org>
Committed: Fri Jan 31 13:52:56 2014 -0500

----------------------------------------------------------------------
 .../java/org/apache/accumulo/fate/AgeOffStore.java  |  1 +
 .../main/java/org/apache/accumulo/fate/Fate.java    |  2 +-
 .../java/org/apache/accumulo/fate/ZooStore.java     | 14 ++++++++------
 .../fate/zookeeper/DistributedReadWriteLock.java    | 16 +++++++++-------
 .../apache/accumulo/fate/zookeeper/ZooCache.java    |  6 ++++--
 .../org/apache/accumulo/fate/zookeeper/ZooLock.java |  9 ++++-----
 .../accumulo/fate/zookeeper/ZooReservation.java     | 16 ++++++++++------
 .../apache/accumulo/fate/zookeeper/ZooSession.java  |  5 ++++-
 8 files changed, 41 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/3bc08e9c/fate/src/main/java/org/apache/accumulo/fate/AgeOffStore.java
----------------------------------------------------------------------
diff --git a/fate/src/main/java/org/apache/accumulo/fate/AgeOffStore.java b/fate/src/main/java/org/apache/accumulo/fate/AgeOffStore.java
index 3f52e70..f90c3a2 100644
--- a/fate/src/main/java/org/apache/accumulo/fate/AgeOffStore.java
+++ b/fate/src/main/java/org/apache/accumulo/fate/AgeOffStore.java
@@ -126,6 +126,7 @@ public class AgeOffStore<T> implements TStore<T> {
           case FAILED:
           case SUCCESSFUL:
             addCandidate(txid);
+            break;
           default:
             break;
         }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/3bc08e9c/fate/src/main/java/org/apache/accumulo/fate/Fate.java
----------------------------------------------------------------------
diff --git a/fate/src/main/java/org/apache/accumulo/fate/Fate.java b/fate/src/main/java/org/apache/accumulo/fate/Fate.java
index 670154e..9d24b0b 100644
--- a/fate/src/main/java/org/apache/accumulo/fate/Fate.java
+++ b/fate/src/main/java/org/apache/accumulo/fate/Fate.java
@@ -170,7 +170,7 @@ public class Fate<T> {
         }
         
         if (autoCleanUp)
-          store.setProperty(tid, AUTO_CLEAN_PROP, new Boolean(autoCleanUp));
+          store.setProperty(tid, AUTO_CLEAN_PROP, Boolean.valueOf(autoCleanUp));
         
         store.setProperty(tid, DEBUG_PROP, repo.getDescription());
         

http://git-wip-us.apache.org/repos/asf/accumulo/blob/3bc08e9c/fate/src/main/java/org/apache/accumulo/fate/ZooStore.java
----------------------------------------------------------------------
diff --git a/fate/src/main/java/org/apache/accumulo/fate/ZooStore.java b/fate/src/main/java/org/apache/accumulo/fate/ZooStore.java
index c78dcc1..5fc1858 100644
--- a/fate/src/main/java/org/apache/accumulo/fate/ZooStore.java
+++ b/fate/src/main/java/org/apache/accumulo/fate/ZooStore.java
@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.io.Serializable;
+import java.nio.charset.Charset;
 import java.security.SecureRandom;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -44,6 +45,7 @@ import org.apache.zookeeper.KeeperException.NodeExistsException;
 //TODO document zookeeper layout - ACCUMULO-1298
 
 public class ZooStore<T> implements TStore<T> {
+  private static final Charset UTF8 = Charset.forName("UTF-8");
   
   private String path;
   private IZooReaderWriter zk;
@@ -103,7 +105,7 @@ public class ZooStore<T> implements TStore<T> {
       try {
         // looking at the code for SecureRandom, it appears to be thread safe
         long tid = idgenerator.nextLong() & 0x7fffffffffffffffl;
-        zk.putPersistentData(getTXPath(tid), TStatus.NEW.name().getBytes(), NodeExistsPolicy.FAIL);
+        zk.putPersistentData(getTXPath(tid), TStatus.NEW.name().getBytes(UTF8), NodeExistsPolicy.FAIL);
         return tid;
       } catch (NodeExistsException nee) {
         // exist, so just try another random #
@@ -157,7 +159,7 @@ public class ZooStore<T> implements TStore<T> {
           // have reserved id, status should not change
           
           try {
-            TStatus status = TStatus.valueOf(new String(zk.getData(path + "/" + txdir, null)));
+            TStatus status = TStatus.valueOf(new String(zk.getData(path + "/" + txdir, null), UTF8));
             if (status == TStatus.IN_PROGRESS || status == TStatus.FAILED_IN_PROGRESS) {
               return tid;
             } else {
@@ -319,7 +321,7 @@ public class ZooStore<T> implements TStore<T> {
   
   private TStatus _getStatus(long tid) {
     try {
-      return TStatus.valueOf(new String(zk.getData(getTXPath(tid), null)));
+      return TStatus.valueOf(new String(zk.getData(getTXPath(tid), null), UTF8));
     } catch (NoNodeException nne) {
       return TStatus.UNKNOWN;
     } catch (Exception e) {
@@ -362,7 +364,7 @@ public class ZooStore<T> implements TStore<T> {
     verifyReserved(tid);
     
     try {
-      zk.putPersistentData(getTXPath(tid), status.name().getBytes(), NodeExistsPolicy.OVERWRITE);
+      zk.putPersistentData(getTXPath(tid), status.name().getBytes(UTF8), NodeExistsPolicy.OVERWRITE);
     } catch (Exception e) {
       throw new RuntimeException(e);
     }
@@ -390,7 +392,7 @@ public class ZooStore<T> implements TStore<T> {
     
     try {
       if (so instanceof String) {
-        zk.putPersistentData(getTXPath(tid) + "/prop_" + prop, ("S " + so).getBytes(), NodeExistsPolicy.OVERWRITE);
+        zk.putPersistentData(getTXPath(tid) + "/prop_" + prop, ("S " + so).getBytes(UTF8), NodeExistsPolicy.OVERWRITE);
       } else {
         byte[] sera = serialize(so);
         byte[] data = new byte[sera.length + 2];
@@ -416,7 +418,7 @@ public class ZooStore<T> implements TStore<T> {
         System.arraycopy(data, 2, sera, 0, sera.length);
         return (Serializable) deserialize(sera);
       } else if (data[0] == 'S') {
-        return new String(data, 2, data.length - 2);
+        return new String(data, 2, data.length - 2, UTF8);
       } else {
         throw new IllegalStateException("Bad property data " + prop);
       }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/3bc08e9c/fate/src/main/java/org/apache/accumulo/fate/zookeeper/DistributedReadWriteLock.java
----------------------------------------------------------------------
diff --git a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/DistributedReadWriteLock.java b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/DistributedReadWriteLock.java
index a9e4f1e..cdf86c0 100644
--- a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/DistributedReadWriteLock.java
+++ b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/DistributedReadWriteLock.java
@@ -16,6 +16,7 @@
  */
 package org.apache.accumulo.fate.zookeeper;
 
+import java.nio.charset.Charset;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.Map.Entry;
@@ -31,6 +32,7 @@ import org.apache.log4j.Logger;
 // A ReadWriteLock that can be implemented in ZooKeeper.  Features the ability to store data
 // with the lock, and recover the lock using that data to find the lock.
 public class DistributedReadWriteLock implements java.util.concurrent.locks.ReadWriteLock {
+  private static final Charset UTF8 = Charset.forName("UTF-8");
   
   static enum LockType {
     READ, WRITE,
@@ -58,7 +60,7 @@ public class DistributedReadWriteLock implements java.util.concurrent.locks.Read
       if (split == -1)
         throw new IllegalArgumentException();
       
-      this.type = LockType.valueOf(new String(lockData, 0, split));
+      this.type = LockType.valueOf(new String(lockData, 0, split, UTF8));
       this.userData = Arrays.copyOfRange(lockData, split + 1, lockData.length);
     }
     
@@ -71,7 +73,7 @@ public class DistributedReadWriteLock implements java.util.concurrent.locks.Read
     }
     
     public byte[] getLockData() {
-      byte typeBytes[] = type.name().getBytes();
+      byte typeBytes[] = type.name().getBytes(UTF8);
       byte[] result = new byte[userData.length + 1 + typeBytes.length];
       System.arraycopy(typeBytes, 0, result, 0, typeBytes.length);
       result[typeBytes.length] = ':';
@@ -143,7 +145,7 @@ public class DistributedReadWriteLock implements java.util.concurrent.locks.Read
     public boolean tryLock() {
       if (entry == -1) {
         entry = qlock.addEntry(new ParsedLock(this.lockType(), this.userData).getLockData());
-        log.info("Added lock entry " + entry + " userData " + new String(this.userData) + " lockType " + lockType());
+        log.info("Added lock entry " + entry + " userData " + new String(this.userData, UTF8) + " lockType " + lockType());
       }
       SortedMap<Long,byte[]> entries = qlock.getEarlierEntries(entry);
       for (Entry<Long,byte[]> entry : entries.entrySet()) {
@@ -153,7 +155,7 @@ public class DistributedReadWriteLock implements java.util.concurrent.locks.Read
         if (parsed.type == LockType.WRITE)
           return false;
       }
-      throw new IllegalStateException("Did not find our own lock in the queue: " + this.entry + " userData " + new String(this.userData) + " lockType "
+      throw new IllegalStateException("Did not find our own lock in the queue: " + this.entry + " userData " + new String(this.userData, UTF8) + " lockType "
           + lockType());
     }
     
@@ -175,7 +177,7 @@ public class DistributedReadWriteLock implements java.util.concurrent.locks.Read
     public void unlock() {
       if (entry == -1)
         return;
-      log.debug("Removing lock entry " + entry + " userData " + new String(this.userData) + " lockType " + lockType());
+      log.debug("Removing lock entry " + entry + " userData " + new String(this.userData, UTF8) + " lockType " + lockType());
       qlock.removeEntry(entry);
       entry = -1;
     }
@@ -205,12 +207,12 @@ public class DistributedReadWriteLock implements java.util.concurrent.locks.Read
     public boolean tryLock() {
       if (entry == -1) {
         entry = qlock.addEntry(new ParsedLock(this.lockType(), this.userData).getLockData());
-        log.info("Added lock entry " + entry + " userData " + new String(this.userData) + " lockType " + lockType());
+        log.info("Added lock entry " + entry + " userData " + new String(this.userData, UTF8) + " lockType " + lockType());
       }
       SortedMap<Long,byte[]> entries = qlock.getEarlierEntries(entry);
       Iterator<Entry<Long,byte[]>> iterator = entries.entrySet().iterator();
       if (!iterator.hasNext())
-        throw new IllegalStateException("Did not find our own lock in the queue: " + this.entry + " userData " + new String(this.userData) + " lockType "
+        throw new IllegalStateException("Did not find our own lock in the queue: " + this.entry + " userData " + new String(this.userData, UTF8) + " lockType "
             + lockType());
       if (iterator.next().getKey().equals(entry))
         return true;

http://git-wip-us.apache.org/repos/asf/accumulo/blob/3bc08e9c/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java
----------------------------------------------------------------------
diff --git a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java
index e57b0b6..637c0e2 100644
--- a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java
+++ b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java
@@ -21,6 +21,7 @@ import java.io.ByteArrayOutputStream;
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
 import java.io.IOException;
+import java.nio.charset.Charset;
 import java.util.Collections;
 import java.util.ConcurrentModificationException;
 import java.util.HashMap;
@@ -42,6 +43,7 @@ import org.apache.zookeeper.data.Stat;
  */
 public class ZooCache {
   private static final Logger log = Logger.getLogger(ZooCache.class);
+  private static final Charset UTF8 = Charset.forName("UTF-8");
 
   private ZCacheWatcher watcher = new ZCacheWatcher();
   private Watcher externalWatcher = null;
@@ -221,10 +223,10 @@ public class ZooCache {
             throw new ConcurrentModificationException();
           }
           if (log.isTraceEnabled())
-            log.trace("zookeeper contained " + zPath + " " + (data == null ? null : new String(data)));
+            log.trace("zookeeper contained " + zPath + " " + (data == null ? null : new String(data, UTF8)));
         }
         if (log.isTraceEnabled())
-          log.trace("putting " + zPath + " " + (data == null ? null : new String(data)) + " in cache");
+          log.trace("putting " + zPath + " " + (data == null ? null : new String(data, UTF8)) + " in cache");
         put(zPath, data, stat);
       }
 

http://git-wip-us.apache.org/repos/asf/accumulo/blob/3bc08e9c/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java
----------------------------------------------------------------------
diff --git a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java
index 8772a83..86c3b88 100644
--- a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java
+++ b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java
@@ -16,6 +16,7 @@
  */
 package org.apache.accumulo.fate.zookeeper;
 
+import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -32,7 +33,7 @@ import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.data.Stat;
 
 public class ZooLock implements Watcher {
-  
+  private static final Charset UTF8 = Charset.forName("UTF-8");
   protected static final Logger log = Logger.getLogger(ZooLock.class);
   
   public static final String LOCK_PREFIX = "zlock-";
@@ -346,9 +347,7 @@ public class ZooLock implements Watcher {
     watchingParent = false;
 
     if (event.getState() == KeeperState.Expired && lock != null) {
-      if (lock != null) {
-        lostLock(LockLossReason.SESSION_EXPIRED);
-      }
+      lostLock(LockLossReason.SESSION_EXPIRED);
     } else {
       
       try { // set the watch on the parent node again
@@ -504,7 +503,7 @@ public class ZooLock implements Watcher {
     Stat stat = new Stat();
     byte[] data = zk.getData(path + "/" + lockNode, stat);
     
-    if (lockData.equals(new String(data))) {
+    if (lockData.equals(new String(data, UTF8))) {
       zk.recursiveDelete(path + "/" + lockNode, stat.getVersion(), NodeMissingPolicy.FAIL);
       return true;
     }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/3bc08e9c/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooReservation.java
----------------------------------------------------------------------
diff --git a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooReservation.java b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooReservation.java
index e938e88..f77260d 100644
--- a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooReservation.java
+++ b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooReservation.java
@@ -16,6 +16,8 @@
  */
 package org.apache.accumulo.fate.zookeeper;
 
+import java.nio.charset.Charset;
+
 import org.apache.accumulo.fate.zookeeper.ZooUtil.NodeExistsPolicy;
 import org.apache.accumulo.fate.zookeeper.ZooUtil.NodeMissingPolicy;
 import org.apache.log4j.Logger;
@@ -25,6 +27,7 @@ import org.apache.zookeeper.KeeperException.NodeExistsException;
 import org.apache.zookeeper.data.Stat;
 
 public class ZooReservation {
+  private static final Charset UTF8 = Charset.forName("UTF-8");
   
   public static boolean attempt(IZooReaderWriter zk, String path, String reservationID, String debugInfo) throws KeeperException, InterruptedException {
     if (reservationID.contains(":"))
@@ -32,7 +35,7 @@ public class ZooReservation {
     
     while (true) {
       try {
-        zk.putPersistentData(path, (reservationID + ":" + debugInfo).getBytes(), NodeExistsPolicy.FAIL);
+        zk.putPersistentData(path, (reservationID + ":" + debugInfo).getBytes(UTF8), NodeExistsPolicy.FAIL);
         return true;
       } catch (NodeExistsException nee) {
         Stat stat = new Stat();
@@ -43,9 +46,9 @@ public class ZooReservation {
           continue;
         }
         
-        String idInZoo = new String(zooData).split(":")[0];
+        String idInZoo = new String(zooData, UTF8).split(":")[0];
         
-        return idInZoo.equals(new String(reservationID));
+        return idInZoo.equals(reservationID);
       }
     }
     
@@ -63,10 +66,11 @@ public class ZooReservation {
       return;
     }
     
-    String idInZoo = new String(zooData).split(":")[0];
+    String zooDataStr = new String(zooData, UTF8);
+    String idInZoo = zooDataStr.split(":")[0];
     
-    if (!idInZoo.equals(new String(reservationID))) {
-      throw new IllegalStateException("Tried to release reservation " + path + " with data mismatch " + new String(reservationID) + " " + new String(zooData));
+    if (!idInZoo.equals(reservationID)) {
+      throw new IllegalStateException("Tried to release reservation " + path + " with data mismatch " + reservationID + " " + zooDataStr);
     }
     
     zk.recursiveDelete(path, stat.getVersion(), NodeMissingPolicy.SKIP);

http://git-wip-us.apache.org/repos/asf/accumulo/blob/3bc08e9c/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java
----------------------------------------------------------------------
diff --git a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java
index 13f6d08..de9729a 100644
--- a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java
+++ b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java
@@ -18,8 +18,10 @@ package org.apache.accumulo.fate.zookeeper;
 
 import java.io.IOException;
 import java.net.UnknownHostException;
+import java.nio.charset.Charset;
 import java.util.HashMap;
 import java.util.Map;
+
 import org.apache.accumulo.fate.util.AddressUtil;
 import org.apache.accumulo.fate.util.UtilWaitThread;
 import org.apache.log4j.Logger;
@@ -30,6 +32,7 @@ import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.ZooKeeper.States;
 
 public class ZooSession {
+  private static final Charset UTF8 = Charset.forName("UTF-8");
   
   public static class ZooSessionShutdownException extends RuntimeException {
 
@@ -50,7 +53,7 @@ public class ZooSession {
   private static Map<String,ZooSessionInfo> sessions = new HashMap<String,ZooSessionInfo>();
   
   private static String sessionKey(String keepers, int timeout, String scheme, byte[] auth) {
-    return keepers + ":" + timeout + ":" + (scheme == null ? "" : scheme) + ":" + (auth == null ? "" : new String(auth));
+    return keepers + ":" + timeout + ":" + (scheme == null ? "" : scheme) + ":" + (auth == null ? "" : new String(auth, UTF8));
   }
   
   private static class ZooWatcher implements Watcher {