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/93207880 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/93207880 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/93207880 Branch: refs/heads/2292-findbugs Commit: 93207880bc438c812006ec1ae4acd56d86c73da5 Parents: 5f416cb Author: Josh Elser <els...@apache.org> Authored: Fri Jan 31 13:52:56 2014 -0500 Committer: Josh Elser <els...@apache.org> Committed: Fri Jan 31 22:18:53 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/93207880/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/93207880/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/93207880/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/93207880/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/93207880/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/93207880/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/93207880/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/93207880/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 {
