Updated Branches: refs/heads/1.4.5-SNAPSHOT 8669b80f9 -> e946ba052
ACCUMULO-1858 revert commits that added ZooKeeperInstance.close() Revert "ACCUMULO-2027 Synchronized access to ZooKeeperInstance methods that mutated state" This reverts commit 975e8c05e8d11f3848e6c800f4d2772026f6c3a3. Revert "ACCUMULO-1984 Rework interruption for instance implementations." This reverts commit 0d0bc4643a8680593e2cf5f828b7566c30fcb345. Conflicts: src/core/src/main/java/org/apache/accumulo/core/client/Instance.java src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooReader.java Revert "ACCUMULO-1889 mark ZKI as closed once close() is called." This reverts commit ada4180379d46297c1531cf8065de5030d12953d. Revert "ACCUMULO-1858 Backport ZooKeeper clean up to 1.4 and 1.5." This reverts commit 79d686faa1e477b9cbd80c6f833ece402050b490. Conflicts: src/core/src/main/java/org/apache/accumulo/core/client/Instance.java src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/e946ba05 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/e946ba05 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/e946ba05 Branch: refs/heads/1.4.5-SNAPSHOT Commit: e946ba052c3fcce8d07815b9daf51bcdc3febbd3 Parents: 8669b80 Author: Keith Turner <ktur...@apache.org> Authored: Thu Jan 2 21:20:43 2014 -0500 Committer: Keith Turner <ktur...@apache.org> Committed: Mon Jan 6 16:13:46 2014 -0500 ---------------------------------------------------------------------- .../apache/accumulo/core/client/Instance.java | 8 +-- .../accumulo/core/client/ZooKeeperInstance.java | 51 +++----------------- .../core/client/impl/ThriftTransportPool.java | 16 ++---- .../accumulo/core/client/mock/MockInstance.java | 5 -- .../apache/accumulo/core/util/ThriftUtil.java | 4 -- .../accumulo/core/zookeeper/ZooCache.java | 7 --- .../accumulo/core/zookeeper/ZooReader.java | 12 ----- .../core/client/impl/TabletLocatorImplTest.java | 5 -- .../accumulo/server/client/HdfsZooInstance.java | 5 -- 9 files changed, 10 insertions(+), 103 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/e946ba05/src/core/src/main/java/org/apache/accumulo/core/client/Instance.java ---------------------------------------------------------------------- diff --git a/src/core/src/main/java/org/apache/accumulo/core/client/Instance.java b/src/core/src/main/java/org/apache/accumulo/core/client/Instance.java index b3ed056..beaed8a 100644 --- a/src/core/src/main/java/org/apache/accumulo/core/client/Instance.java +++ b/src/core/src/main/java/org/apache/accumulo/core/client/Instance.java @@ -126,13 +126,7 @@ public interface Instance { * when a user's credentials are invalid */ public abstract Connector getConnector(String user, CharSequence pass) throws AccumuloException, AccumuloSecurityException; - - /** - * Closes up the instance to free up all associated resources. You should try to reuse an Instance as much as you can because there is some location caching - * stored which will enhance performance. - */ - public abstract void close(); - + /** * Returns the AccumuloConfiguration to use when interacting with this instance. * http://git-wip-us.apache.org/repos/asf/accumulo/blob/e946ba05/src/core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java ---------------------------------------------------------------------- diff --git a/src/core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java b/src/core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java index 5016acb..e02c197 100644 --- a/src/core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java +++ b/src/core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java @@ -22,7 +22,6 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; import java.util.UUID; -import java.util.concurrent.atomic.AtomicInteger; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.client.impl.ConnectorImpl; @@ -35,7 +34,6 @@ import org.apache.accumulo.core.util.ByteBufferUtil; import org.apache.accumulo.core.util.CachedConfiguration; import org.apache.accumulo.core.util.OpTimer; import org.apache.accumulo.core.util.TextUtil; -import org.apache.accumulo.core.util.ThriftUtil; import org.apache.accumulo.core.zookeeper.ZooCache; import org.apache.accumulo.core.zookeeper.ZooUtil; import org.apache.hadoop.fs.FileStatus; @@ -72,8 +70,6 @@ public class ZooKeeperInstance implements Instance { private int zooKeepersSessionTimeOut; - private volatile boolean closed = false; - /** * * @param instanceName @@ -103,7 +99,6 @@ public class ZooKeeperInstance implements Instance { this.zooKeepersSessionTimeOut = sessionTimeout; zooCache = ZooCache.getInstance(zooKeepers, sessionTimeout); getInstanceID(); - clientInstances.incrementAndGet(); } /** @@ -134,13 +129,10 @@ public class ZooKeeperInstance implements Instance { this.zooKeepers = zooKeepers; this.zooKeepersSessionTimeOut = sessionTimeout; zooCache = ZooCache.getInstance(zooKeepers, sessionTimeout); - clientInstances.incrementAndGet(); } @Override - public synchronized String getInstanceID() { - if (closed) - throw new RuntimeException("ZooKeeperInstance has been closed."); + public String getInstanceID() { if (instanceId == null) { // want the instance id to be stable for the life of this instance object, // so only get it once @@ -163,9 +155,7 @@ public class ZooKeeperInstance implements Instance { } @Override - public synchronized List<String> getMasterLocations() { - if (closed) - throw new RuntimeException("ZooKeeperInstance has been closed."); + public List<String> getMasterLocations() { String masterLocPath = ZooUtil.getRoot(this) + Constants.ZMASTER_LOCK; OpTimer opTimer = new OpTimer(log, Level.TRACE).start("Looking up master location in zoocache."); @@ -180,9 +170,7 @@ public class ZooKeeperInstance implements Instance { } @Override - public synchronized String getRootTabletLocation() { - if (closed) - throw new RuntimeException("ZooKeeperInstance has been closed."); + public String getRootTabletLocation() { String zRootLocPath = ZooUtil.getRoot(this) + Constants.ZROOT_TABLET_LOCATION; OpTimer opTimer = new OpTimer(log, Level.TRACE).start("Looking up root tablet location in zookeeper."); @@ -197,9 +185,7 @@ public class ZooKeeperInstance implements Instance { } @Override - public synchronized String getInstanceName() { - if (closed) - throw new RuntimeException("ZooKeeperInstance has been closed."); + public String getInstanceName() { if (instanceName == null) instanceName = lookupInstanceName(zooCache, UUID.fromString(getInstanceID())); @@ -230,8 +216,6 @@ public class ZooKeeperInstance implements Instance { @SuppressWarnings("deprecation") @Override public Connector getConnector(String user, byte[] pass) throws AccumuloException, AccumuloSecurityException { - if (closed) - throw new RuntimeException("ZooKeeperInstance has been closed."); return new ConnectorImpl(this, user, pass); } @@ -268,7 +252,7 @@ public class ZooKeeperInstance implements Instance { } return null; } - + // To be moved to server code. Only lives here to support the Accumulo Shell @Deprecated public static String getInstanceIDFromHdfs(Path instanceDirectory) { @@ -295,32 +279,9 @@ public class ZooKeeperInstance implements Instance { throw new RuntimeException("Accumulo not initialized, there is no instance id at " + instanceDirectory, e); } } - + @Override public Connector getConnector(AuthInfo auth) throws AccumuloException, AccumuloSecurityException { return getConnector(auth.user, auth.password); } - - static private final AtomicInteger clientInstances = new AtomicInteger(0); - - @Override - public synchronized void close() { - if (!closed && clientInstances.decrementAndGet() == 0) { - try { - zooCache.close(); - ThriftUtil.close(); - } catch (RuntimeException e) { - clientInstances.incrementAndGet(); - throw e; - } - } - closed = true; - } - - @Override - public void finalize() { - // This method intentionally left blank. Users need to explicitly close Instances if they want things cleaned up nicely. - if (!closed) - log.warn("ZooKeeperInstance being cleaned up without being closed. Please remember to call close() before dereferencing to clean up threads."); - } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/e946ba05/src/core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportPool.java ---------------------------------------------------------------------- diff --git a/src/core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportPool.java b/src/core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportPool.java index f969f28..ef3724b 100644 --- a/src/core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportPool.java +++ b/src/core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportPool.java @@ -80,15 +80,13 @@ public class ThriftTransportPool { private static class Closer implements Runnable { ThriftTransportPool pool; - final AtomicBoolean stop; - public Closer(ThriftTransportPool pool, AtomicBoolean stop) { + public Closer(ThriftTransportPool pool) { this.pool = pool; - this.stop = stop; } public void run() { - while (!stop.get()) { + while (true) { ArrayList<CachedConnection> connectionsToClose = new ArrayList<CachedConnection>(); @@ -594,7 +592,6 @@ public class ThriftTransportPool { private static ThriftTransportPool instance = new ThriftTransportPool(); private static final AtomicBoolean daemonStarted = new AtomicBoolean(false); - private static AtomicBoolean stopDaemon; public static ThriftTransportPool getInstance() { SecurityManager sm = System.getSecurityManager(); @@ -603,15 +600,8 @@ public class ThriftTransportPool { } if (daemonStarted.compareAndSet(false, true)) { - stopDaemon = new AtomicBoolean(false); - new Daemon(new Closer(instance, stopDaemon), "Thrift Connection Pool Checker").start(); + new Daemon(new Closer(instance), "Thrift Connection Pool Checker").start(); } return instance; } - - public static void close() { - if (daemonStarted.compareAndSet(true, false)) { - stopDaemon.set(true); - } - } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/e946ba05/src/core/src/main/java/org/apache/accumulo/core/client/mock/MockInstance.java ---------------------------------------------------------------------- diff --git a/src/core/src/main/java/org/apache/accumulo/core/client/mock/MockInstance.java b/src/core/src/main/java/org/apache/accumulo/core/client/mock/MockInstance.java index b9778a7..2ff7b82 100644 --- a/src/core/src/main/java/org/apache/accumulo/core/client/mock/MockInstance.java +++ b/src/core/src/main/java/org/apache/accumulo/core/client/mock/MockInstance.java @@ -140,9 +140,4 @@ public class MockInstance implements Instance { public Connector getConnector(AuthInfo auth) throws AccumuloException, AccumuloSecurityException { return getConnector(auth.user, auth.password); } - - @Override - public void close() { - // NOOP - } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/e946ba05/src/core/src/main/java/org/apache/accumulo/core/util/ThriftUtil.java ---------------------------------------------------------------------- diff --git a/src/core/src/main/java/org/apache/accumulo/core/util/ThriftUtil.java b/src/core/src/main/java/org/apache/accumulo/core/util/ThriftUtil.java index 3684ecd..1b1cdd7 100644 --- a/src/core/src/main/java/org/apache/accumulo/core/util/ThriftUtil.java +++ b/src/core/src/main/java/org/apache/accumulo/core/util/ThriftUtil.java @@ -165,8 +165,4 @@ public class ThriftUtil { public static TProtocolFactory protocolFactory() { return protocolFactory; } - - public static void close() { - ThriftTransportPool.close(); - } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/e946ba05/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java ---------------------------------------------------------------------- diff --git a/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java b/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java index cbb3918..f7447ef 100644 --- a/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java +++ b/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java @@ -307,11 +307,4 @@ public class ZooCache { return zc; } - - public void close() { - cache.clear(); - statCache.clear(); - childrenCache.clear(); - zReader.close(); - } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/e946ba05/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooReader.java ---------------------------------------------------------------------- diff --git a/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooReader.java b/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooReader.java index aabc0f2..f1ca363 100644 --- a/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooReader.java +++ b/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooReader.java @@ -29,7 +29,6 @@ import org.apache.zookeeper.ZooKeeper; import org.apache.zookeeper.data.Stat; public class ZooReader implements IZooReader { - protected String keepers; protected int timeout; @@ -108,15 +107,4 @@ public class ZooReader implements IZooReader { public ZooReader(Instance instance) { this(instance.getZooKeepers(), instance.getZooKeepersSessionTimeOut()); } - - /** - * Closes this reader. If closure of the underlying session is interrupted, this method sets the calling thread's interrupt status. - */ - public void close() { - try { - getZooKeeper().close(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/e946ba05/src/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java ---------------------------------------------------------------------- diff --git a/src/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java b/src/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java index e0ae60e..538cb6c 100644 --- a/src/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java +++ b/src/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java @@ -448,11 +448,6 @@ public class TabletLocatorImplTest extends TestCase { public Connector getConnector(AuthInfo auth) throws AccumuloException, AccumuloSecurityException { return getConnector(auth.user, auth.password); } - - @Override - public void close() { - // NOOP - } } static class TServers { http://git-wip-us.apache.org/repos/asf/accumulo/blob/e946ba05/src/server/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java ---------------------------------------------------------------------- diff --git a/src/server/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java b/src/server/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java index 2dd1db6..e6cdb63 100644 --- a/src/server/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java +++ b/src/server/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java @@ -177,11 +177,6 @@ public class HdfsZooInstance implements Instance { System.out.println("ZooKeepers: " + instance.getZooKeepers()); System.out.println("Masters: " + StringUtil.join(instance.getMasterLocations(), ", ")); } - - @Override - public void close() { - zooCache.close(); - } @Override public Connector getConnector(AuthInfo auth) throws AccumuloException, AccumuloSecurityException {