Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Plain View
Accumulo >> mail # dev >> Review Request 16485: ACCUMULO-2103 - Better ZooSession management


+
Bill Havanki 2013-12-27, 19:43
+
Bill Havanki 2013-12-27, 22:02
Copy link to this message
-
Re: Review Request 16485: ACCUMULO-2103 - Better ZooSession management

-----------------------------------------------------------
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.