[ 
https://issues.apache.org/jira/browse/SLIDER-1166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15469317#comment-15469317
 ] 

Josh Elser commented on SLIDER-1166:
------------------------------------

{code}
+  public synchronized boolean init() throws IOException {
+    if (zookeeper != null && getAlive()) {
+      return true;
+    }
+
+    synchronized (zkSessions) {
+      if (zkSessions.containsKey(zkConnection)) {
+        zookeeper = zkSessions.get(zkConnection);
+      }
{code}

{{synchronized (zkSessions)}} looks unnecessary to me. You already have 
guaranteed that you are the only accessor to this method (by marking 
{{synchronized}} on the method).

I'm a little concerned about the close() logic. If there are multiple instances 
of ZKIntegration for the same quorum, one of the instances gets closed, I think 
it would invalidate all of the other instances (obligatory, I have not looked 
at the complete lifecycle, so maybe this isn't a concern?). This is definitely 
a step-up over what is there presently, though :)

> Every cluster stop operation creates and holds on to a zk session
> -----------------------------------------------------------------
>
>                 Key: SLIDER-1166
>                 URL: https://issues.apache.org/jira/browse/SLIDER-1166
>             Project: Slider
>          Issue Type: Bug
>          Components: client
>    Affects Versions: Slider 0.91
>            Reporter: Billie Rinaldi
>            Assignee: Billie Rinaldi
>            Priority: Critical
>             Fix For: Slider 1.0.0
>
>         Attachments: SLIDER-1166.1.patch, SLIDER-1166.2.patch, 
> SLIDER-1166.3.patch
>
>
> We aren't closing the zookeeper client, leaving it to expire and keep the 
> session open much longer than is necessary.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to