accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ktur...@apache.org
Subject [1/4] git commit: ACCUMULO-1858 revert commits that added ZooKeeperInstance.close()
Date Mon, 06 Jan 2014 22:36:16 GMT
Updated Branches:
  refs/heads/1.6.0-SNAPSHOT 016f3bb10 -> f17661cd1


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.6.0-SNAPSHOT
Commit: e946ba052c3fcce8d07815b9daf51bcdc3febbd3
Parents: 8669b80
Author: Keith Turner <kturner@apache.org>
Authored: Thu Jan 2 21:20:43 2014 -0500
Committer: Keith Turner <kturner@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
{


Mime
View raw message