----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16485/#review30923 -----------------------------------------------------------
src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java <https://reviews.apache.org/r/16485/#comment59154> Should, at a minimum, also set Thread.currentThread().interrupt() flag. Also, if this FIXME persists, please create a JIRA for it and reference it here. src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooKeeperFactory.java <https://reviews.apache.org/r/16485/#comment59156> Should this be public? src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooKeeperFactoryException.java <https://reviews.apache.org/r/16485/#comment59155> I think Eclipse will complain about a SerialVersionUid. src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooReader.java <https://reviews.apache.org/r/16485/#comment59157> Synchronization issue? What happens when two threads attempt to access the first ZooKeeper at the same time? src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooSession.java <https://reviews.apache.org/r/16485/#comment59158> Can this leak unclosed sessions? src/core/src/test/java/org/apache/accumulo/core/zookeeper/ZooSessionTest.java <https://reviews.apache.org/r/16485/#comment59159> Should verify that it didn't close after this. - Mike Drob On Dec. 27, 2013, 7:43 p.m., Bill Havanki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16485/ > ----------------------------------------------------------- > > (Updated Dec. 27, 2013, 7:43 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2103 > https://issues.apache.org/jira/browse/ACCUMULO-2103 > > > Repository: accumulo > > > Description > ------- > > This is the work on ZooSession to better support closure of ZK connections > under ACCUMULO-2026 and/or future work. Here's a tour. > > First, the logic in ZooSession to connect to ZooKeeper was moved to a new > ZooKeeperFactory class. This also makes it possible to mock the logic for > unit tests, or swap in newer / better implementations later. The factory will > throw a new ZooKeeperFactoryException (unchecked) for most problems, but does > throw InterruptedException when appropriate (instead of eating it). > Otherwise, the logic is the same. > > Before this, ZooSession was a set of static methods oriented around > connecting to ZooKeeper. With this set of commits, ZooSession objects are > used, each of which contains a ZooKeeper connection object as well as its key > (formed and used as before). > > The ZooSession class still uses a static map of known sessions, so that the > same connection may be reused by several clients. The map now also remembers > the watcher object associated with the connection (possibly a prior > oversight), and a reference count. Whenever a new connection is established > or it is reused, the reference count is increased. When a session is closed, > the reference count is decreased, and the connection itself is only closed > when the reference count is depleted. Cleanup of closed session objects > occurs as it did before, lazily upon request of a new one (see the > removeIfClosed method). > > The getSession methods are deprecated since they return the actual ZK > connection objects instead of sessions. In their place are open methods. The > deprecated methods do not participate in reference counting. > > ZooSession now includes a clear method for simply wiping out its session > tracking and a forceCloseAll method for forcibly closing all known > connections. These methods are useful for ensuring that connections are wiped > clean, so that it isn't necessary to hunt down and close all sessions before, > say, a process exits. > > ZooReader was modified to remember the session that it is using (with its > reference to the ZK connection) instead of continuously asking ZooSession for > it. When a ZooReader is closed, it closes its session instead of closing the > ZK connection itself; this allows the reference counting to work. > > This set of commits adds a unit test for ZooSession. ZooSession is defined to > implement Closeable, but ZooReader is not. > > > Diffs > ----- > > src/core/pom.xml 77a4a72b3d32283e82d7a9d3b1b6b4690520c88e > src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java > cbb39182e93c568363a58d7e3344b9b85337cfc0 > > src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooKeeperFactory.java > PRE-CREATION > > src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooKeeperFactoryException.java > PRE-CREATION > src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooReader.java > aabc0f223bf2c6187e915b9a1e9569d714bce852 > src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooSession.java > b3db26f107139c1e2854a545e3f1f11de1b28b02 > > src/core/src/test/java/org/apache/accumulo/core/zookeeper/ZooSessionTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/16485/diff/ > > > Testing > ------- > > Unit tests; functional tests; continuous ingest. > > > Thanks, > > Bill Havanki > >
