You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@livy.apache.org by GitBox <gi...@apache.org> on 2020/01/19 13:21:01 UTC

[GitHub] [incubator-livy] yiheng opened a new pull request #276: [LIVY-723] Server Registration

yiheng opened a new pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276
 
 
   ## What changes were proposed in this pull request?
   In this patch, we check in a ClusterManager class to handle the node join/leave cluster events based on Zookeeper.
   
   Users can add listeners to monitor node join/leave.
   
   ## How was this patch tested?
   Add new unit tests and manual tests.
   
   Please review https://livy.incubator.apache.org/community/ before opening a pull request.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r391523576
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/cluster/ClusterManager.scala
 ##########
 @@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.livy.cluster
+
+import scala.collection.mutable.Set
+
+case class ServiceNode(host: String, port: Int, UUID: String)
+
+/**
+ * Interface for cluster management.
+ */
+abstract class ClusterManager {
+  /**
+   * Register current node into the cluster. It should be invoked when the node is ready
+   * for service.
+   */
+  def register(): Unit
+
+  /**
+   * Get the nodes in the cluster.
+   * @return
+   */
+  def getNodes(): Set[ServiceNode]
 
 Review comment:
   done

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng commented on issue #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng commented on issue #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#issuecomment-576155883
 
 
   @runzhiwang and @jerryshao Please help review

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r391523523
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/server/recovery/ZooKeeperManager.scala
 ##########
 @@ -115,4 +118,49 @@ class ZooKeeperManager(
       case _: NoNodeException => warn(s"Fail to remove non-existed zookeeper node: ${key}")
     }
   }
+
+  def watchAddNode[T: ClassTag](path: String, nodeAddHandler: (String, T) => Unit): Unit = {
+    watchNode(path, nodeAddHandler, Type.CHILD_ADDED)
+  }
+
+  def watchRemoveNode[T: ClassTag](path: String, nodeRemoveHandler: (String, T) => Unit): Unit = {
+    watchNode(path, nodeRemoveHandler, Type.CHILD_REMOVED)
+  }
+
+  def createEphemeralNode(path: String, value: Object): Unit = {
+    deleteNode(path)
+    val data = serializeToBytes(value)
+    curatorClient.create.creatingParentsIfNeeded.withMode(CreateMode.EPHEMERAL).forPath(path, data)
+  }
+
+  // For test
+  protected def getPathChildrenCache(path: String): PathChildrenCache = {
+    new PathChildrenCache(curatorClient, path, true)
+  }
+
+
 
 Review comment:
   done

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r391523634
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/LivyConf.scala
 ##########
 @@ -186,6 +186,22 @@ object LivyConf {
    */
   val RECOVERY_MODE = Entry("livy.server.recovery.mode", "off")
 
+  /**
+   * HA mode of Livy. Possible values:
+   * 1. null: Default. livy will use the value of livy.server.recovery.mode
+   * 2. off: Turn off recovery. Every time Livy shuts down, it stops and forgets all sessions.
+   * 3. recovery: Livy persists session info to the state store. When Livy restarts, it recovers
+   *              previous sessions from the state store.
+   *              Must set livy.server.recovery.state-store and
+   *              livy.server.recovery.state-store.url to configure the state store.
+   * 4. multi-active: HA with multi-active mode.
+   */
+  val HA_MODE = Entry("livy.server.ha.mode", HA_MODE_OFF)
+  val HA_MODE_OFF = "off"
+  val HA_MODE_RECOVERY = "recovery"
+  val HA_MODE_MULTI_ACTIVE = "multi-active"
+
+
 
 Review comment:
   done

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r391523769
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/cluster/ClusterManager.scala
 ##########
 @@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.livy.cluster
+
+import scala.collection.mutable.Set
+
+case class ServiceNode(host: String, port: Int, UUID: String)
+
+/**
+ * Interface for cluster management.
+ */
+abstract class ClusterManager {
+  /**
+   * Register current node into the cluster. It should be invoked when the node is ready
+   * for service.
+   */
+  def register(): Unit
+
+  /**
+   * Get the nodes in the cluster.
+   * @return
+   */
+  def getNodes(): Set[ServiceNode]
+
+  /**
+   * Add a listener which will be notified when a new node join the cluster.
+   * @param listener
+   */
+  def registerNodeJoinListener(listener: ServiceNode => Unit): Unit
 
 Review comment:
   done

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r391523719
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/LivyConf.scala
 ##########
 @@ -227,6 +243,8 @@ object LivyConf {
   val RECOVERY_ZK_STATE_STORE_KEY_PREFIX =
     Entry("livy.server.recovery.zk-state-store.key-prefix", "livy")
 
+  val ZK_SERVICE_DIR = Entry("livy.server.zk.services", "/livy/zk/services")
 
 Review comment:
   done

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] jerryshao commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
jerryshao commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r368779788
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/LivyConf.scala
 ##########
 @@ -186,6 +186,22 @@ object LivyConf {
    */
   val RECOVERY_MODE = Entry("livy.server.recovery.mode", "off")
 
+  /**
+   * HA mode of Livy. Possible values:
+   * 1. null: Default. livy will use the value of livy.server.recovery.mode
+   * 2. off: Turn off recovery. Every time Livy shuts down, it stops and forgets all sessions.
+   * 3. recovery: Livy persists session info to the state store. When Livy restarts, it recovers
+   *              previous sessions from the state store.
+   *              Must set livy.server.recovery.state-store and
+   *              livy.server.recovery.state-store.url to configure the state store.
+   * 4. multi-active: HA with multi-active mode.
+   */
+  val HA_MODE = Entry("livy.server.ha.mode", HA_MODE_OFF)
+  val HA_MODE_OFF = "off"
+  val HA_MODE_RECOVERY = "recovery"
+  val HA_MODE_MULTI_ACTIVE = "multi-active"
+
+
 
 Review comment:
   nit: additional line.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] jerryshao commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
jerryshao commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r368793046
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/cluster/ClusterManager.scala
 ##########
 @@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.livy.cluster
+
+import scala.collection.mutable.Set
+
+case class ServiceNode(host: String, port: Int, UUID: String)
+
+/**
+ * Interface for cluster management.
+ */
+abstract class ClusterManager {
+  /**
+   * Register current node into the cluster. It should be invoked when the node is ready
+   * for service.
+   */
+  def register(): Unit
+
+  /**
+   * Get the nodes in the cluster.
+   * @return
+   */
+  def getNodes(): Set[ServiceNode]
 
 Review comment:
   Is it better to return an immutable set?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] codecov-io edited a comment on issue #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#issuecomment-576107327
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=h1) Report
   > Merging [#276](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-livy/commit/66b5833e413bc10e39e3b92b585f496444c147d4?src=pr&el=desc) will **increase** coverage by `0.11%`.
   > The diff coverage is `74.07%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-livy/pull/276/graphs/tree.svg?width=650&token=0MkVbiUFwE&height=150&src=pr)](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #276      +/-   ##
   ============================================
   + Coverage     68.14%   68.26%   +0.11%     
   - Complexity      959      980      +21     
   ============================================
     Files           104      106       +2     
     Lines          5946     5999      +53     
     Branches        899      908       +9     
   ============================================
   + Hits           4052     4095      +43     
   - Misses         1312     1313       +1     
   - Partials        582      591       +9
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...rver/src/main/scala/org/apache/livy/LivyConf.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9MaXZ5Q29uZi5zY2FsYQ==) | `96.22% <100%> (+0.09%)` | `21 <0> (ø)` | :arrow_down: |
   | [...main/scala/org/apache/livy/server/LivyServer.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvTGl2eVNlcnZlci5zY2FsYQ==) | `33.18% <33.33%> (-0.3%)` | `11 <0> (ø)` | |
   | [...scala/org/apache/livy/cluster/ClusterManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9jbHVzdGVyL0NsdXN0ZXJNYW5hZ2VyLnNjYWxh) | `50% <50%> (ø)` | `1 <1> (?)` | |
   | [...apache/livy/server/recovery/ZooKeeperManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvcmVjb3ZlcnkvWm9vS2VlcGVyTWFuYWdlci5zY2FsYQ==) | `67.24% <68.75%> (+0.57%)` | `22 <5> (+5)` | :arrow_up: |
   | [...ala/org/apache/livy/cluster/ZKClusterManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9jbHVzdGVyL1pLQ2x1c3Rlck1hbmFnZXIuc2NhbGE=) | `84% <84%> (ø)` | `14 <14> (?)` | |
   | [...ain/java/org/apache/livy/rsc/driver/RSCDriver.java](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-cnNjL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9saXZ5L3JzYy9kcml2ZXIvUlNDRHJpdmVyLmphdmE=) | `81.66% <0%> (+1.66%)` | `45% <0%> (ø)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=footer). Last update [66b5833...7b6b2a8](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] jerryshao commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
jerryshao commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r368792814
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/server/recovery/ZooKeeperManager.scala
 ##########
 @@ -115,4 +118,49 @@ class ZooKeeperManager(
       case _: NoNodeException => warn(s"Fail to remove non-existed zookeeper node: ${key}")
     }
   }
+
+  def watchAddNode[T: ClassTag](path: String, nodeAddHandler: (String, T) => Unit): Unit = {
+    watchNode(path, nodeAddHandler, Type.CHILD_ADDED)
+  }
+
+  def watchRemoveNode[T: ClassTag](path: String, nodeRemoveHandler: (String, T) => Unit): Unit = {
+    watchNode(path, nodeRemoveHandler, Type.CHILD_REMOVED)
+  }
+
+  def createEphemeralNode(path: String, value: Object): Unit = {
+    deleteNode(path)
+    val data = serializeToBytes(value)
+    curatorClient.create.creatingParentsIfNeeded.withMode(CreateMode.EPHEMERAL).forPath(path, data)
+  }
+
+  // For test
+  protected def getPathChildrenCache(path: String): PathChildrenCache = {
+    new PathChildrenCache(curatorClient, path, true)
+  }
+
+
 
 Review comment:
   nit: additional line.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r391523456
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/cluster/ZKClusterManager.scala
 ##########
 @@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.livy.cluster
+
+import java.util.UUID
+
+import scala.collection.mutable
+import scala.collection.mutable.{ArrayBuffer, Set}
+
+import org.apache.livy.rsc.RSCConf.Entry.LAUNCHER_ADDRESS
+import org.apache.livy.LivyConf.{SERVER_PORT, ZK_SERVICE_DIR}
+import org.apache.livy.LivyConf
+import org.apache.livy.Logging
+import org.apache.livy.server.recovery.ZooKeeperManager
+
+class ZKClusterManager(livyConf: LivyConf, zkManager: ZooKeeperManager)
+  extends ClusterManager with Logging {
+  private val serverIP = livyConf.get(LAUNCHER_ADDRESS)
+  require(serverIP != null, "Please config the livy.rsc.launcher.address")
+
+  private val port = livyConf.getInt(SERVER_PORT)
+  private val serviceDir = livyConf.get(ZK_SERVICE_DIR)
+
+  private val nodes = new mutable.HashSet[ServiceNode]()
+  private val nodeJoinListeners = new ArrayBuffer[ServiceNode => Unit]()
+  private val nodeLeaveListeners = new ArrayBuffer[ServiceNode => Unit]()
+
+  zkManager.getChildren(serviceDir).foreach(node => {
+    val serviceNode = zkManager.get[ServiceNode](serviceDir + "/" + node).get
+    nodes.add(serviceNode)
+  })
+
+  // Start listening
+  zkManager.watchAddNode(serviceDir, nodeAddHandler)
+  zkManager.watchRemoveNode(serviceDir, nodeRemoveHandler)
+
+  override def register(): Unit = {
+    val node = ServiceNode(serverIP, port, UUID.randomUUID().toString)
+    zkManager.createEphemeralNode(serviceDir + "/" + serverIP + ":" + port, node)
+  }
+
+  override def getNodes(): Set[ServiceNode] = {
+    nodes
+  }
+
+  override def registerNodeJoinListener(listener: ServiceNode => Unit): Unit = {
+    nodeJoinListeners.append(listener)
+  }
+
+  override def registerNodeLeaveListener(listener : ServiceNode => Unit): Unit = {
+    nodeLeaveListeners.append(listener)
+  }
+
+  private def nodeAddHandler(path: String, node: ServiceNode): Unit = {
+    logger.info("Detect new node join: " + node)
+    nodes.add(node)
+    nodeJoinListeners.foreach(_(node))
+  }
+
+  private def nodeRemoveHandler(path: String, node: ServiceNode): Unit = {
+    logger.info("Detect node leave: " + node)
+    nodes.remove(node)
+    nodeLeaveListeners.foreach(_(node))
 
 Review comment:
   Here we notify all registered listeners on the server. The listenser may take action based on the node join/leave event.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng opened a new pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng opened a new pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276
 
 
   ## What changes were proposed in this pull request?
   In this patch, we check in a ClusterManager class to handle the node join/leave cluster events based on Zookeeper.
   
   Users can add listeners to monitor node join/leave.
   
   ## How was this patch tested?
   Add new unit tests and manual tests.
   
   Please review https://livy.incubator.apache.org/community/ before opening a pull request.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] codecov-io edited a comment on issue #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#issuecomment-576107327
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=h1) Report
   > Merging [#276](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-livy/commit/66b5833e413bc10e39e3b92b585f496444c147d4?src=pr&el=desc) will **increase** coverage by `0.19%`.
   > The diff coverage is `74.07%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-livy/pull/276/graphs/tree.svg?width=650&token=0MkVbiUFwE&height=150&src=pr)](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #276      +/-   ##
   ============================================
   + Coverage     68.14%   68.34%   +0.19%     
   - Complexity      959      984      +25     
   ============================================
     Files           104      106       +2     
     Lines          5946     6005      +59     
     Branches        899      909      +10     
   ============================================
   + Hits           4052     4104      +52     
     Misses         1312     1312              
   - Partials        582      589       +7
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...rver/src/main/scala/org/apache/livy/LivyConf.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9MaXZ5Q29uZi5zY2FsYQ==) | `96.22% <100%> (+0.09%)` | `21 <0> (ø)` | :arrow_down: |
   | [...main/scala/org/apache/livy/server/LivyServer.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvTGl2eVNlcnZlci5zY2FsYQ==) | `33.18% <33.33%> (-0.3%)` | `11 <0> (ø)` | |
   | [...scala/org/apache/livy/cluster/ClusterManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9jbHVzdGVyL0NsdXN0ZXJNYW5hZ2VyLnNjYWxh) | `50% <50%> (ø)` | `1 <1> (?)` | |
   | [...apache/livy/server/recovery/ZooKeeperManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvcmVjb3ZlcnkvWm9vS2VlcGVyTWFuYWdlci5zY2FsYQ==) | `67.24% <68.75%> (+0.57%)` | `22 <5> (+5)` | :arrow_up: |
   | [...ala/org/apache/livy/cluster/ZKClusterManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9jbHVzdGVyL1pLQ2x1c3Rlck1hbmFnZXIuc2NhbGE=) | `84% <84%> (ø)` | `14 <14> (?)` | |
   | [...c/main/scala/org/apache/livy/repl/ReplDriver.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-cmVwbC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2xpdnkvcmVwbC9SZXBsRHJpdmVyLnNjYWxh) | `30.76% <0%> (-2.57%)` | `7% <0%> (ø)` | |
   | [...va/org/apache/livy/client/http/LivyConnection.java](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-Y2xpZW50LWh0dHAvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2xpdnkvY2xpZW50L2h0dHAvTGl2eUNvbm5lY3Rpb24uamF2YQ==) | `82.27% <0%> (+0.22%)` | `15% <0%> (ø)` | :arrow_down: |
   | ... and [3 more](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=footer). Last update [66b5833...62b09aa](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] codecov-io edited a comment on issue #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#issuecomment-576107327
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=h1) Report
   > Merging [#276](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-livy/commit/66b5833e413bc10e39e3b92b585f496444c147d4?src=pr&el=desc) will **increase** coverage by `0.19%`.
   > The diff coverage is `74.07%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-livy/pull/276/graphs/tree.svg?width=650&token=0MkVbiUFwE&height=150&src=pr)](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #276      +/-   ##
   ============================================
   + Coverage     68.14%   68.34%   +0.19%     
   - Complexity      959      984      +25     
   ============================================
     Files           104      106       +2     
     Lines          5946     6005      +59     
     Branches        899      909      +10     
   ============================================
   + Hits           4052     4104      +52     
     Misses         1312     1312              
   - Partials        582      589       +7
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...rver/src/main/scala/org/apache/livy/LivyConf.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9MaXZ5Q29uZi5zY2FsYQ==) | `96.22% <100%> (+0.09%)` | `21 <0> (ø)` | :arrow_down: |
   | [...main/scala/org/apache/livy/server/LivyServer.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvTGl2eVNlcnZlci5zY2FsYQ==) | `33.18% <33.33%> (-0.3%)` | `11 <0> (ø)` | |
   | [...scala/org/apache/livy/cluster/ClusterManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9jbHVzdGVyL0NsdXN0ZXJNYW5hZ2VyLnNjYWxh) | `50% <50%> (ø)` | `1 <1> (?)` | |
   | [...apache/livy/server/recovery/ZooKeeperManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvcmVjb3ZlcnkvWm9vS2VlcGVyTWFuYWdlci5zY2FsYQ==) | `67.24% <68.75%> (+0.57%)` | `22 <5> (+5)` | :arrow_up: |
   | [...ala/org/apache/livy/cluster/ZKClusterManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9jbHVzdGVyL1pLQ2x1c3Rlck1hbmFnZXIuc2NhbGE=) | `84% <84%> (ø)` | `14 <14> (?)` | |
   | [...c/main/scala/org/apache/livy/repl/ReplDriver.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-cmVwbC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2xpdnkvcmVwbC9SZXBsRHJpdmVyLnNjYWxh) | `30.76% <0%> (-2.57%)` | `7% <0%> (ø)` | |
   | [...va/org/apache/livy/client/http/LivyConnection.java](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-Y2xpZW50LWh0dHAvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2xpdnkvY2xpZW50L2h0dHAvTGl2eUNvbm5lY3Rpb24uamF2YQ==) | `82.27% <0%> (+0.22%)` | `15% <0%> (ø)` | :arrow_down: |
   | ... and [3 more](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=footer). Last update [66b5833...62b09aa](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r391523576
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/cluster/ClusterManager.scala
 ##########
 @@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.livy.cluster
+
+import scala.collection.mutable.Set
+
+case class ServiceNode(host: String, port: Int, UUID: String)
+
+/**
+ * Interface for cluster management.
+ */
+abstract class ClusterManager {
+  /**
+   * Register current node into the cluster. It should be invoked when the node is ready
+   * for service.
+   */
+  def register(): Unit
+
+  /**
+   * Get the nodes in the cluster.
+   * @return
+   */
+  def getNodes(): Set[ServiceNode]
 
 Review comment:
   done

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng commented on issue #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng commented on issue #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#issuecomment-605750117
 
 
   ping @jerryshao .Could you help reivew it again?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] jerryshao commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
jerryshao commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r368780706
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/cluster/ClusterManager.scala
 ##########
 @@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.livy.cluster
+
+import scala.collection.mutable.Set
+
+case class ServiceNode(host: String, port: Int, UUID: String)
+
+/**
+ * Interface for cluster management.
+ */
+abstract class ClusterManager {
+  /**
+   * Register current node into the cluster. It should be invoked when the node is ready
+   * for service.
+   */
+  def register(): Unit
+
+  /**
+   * Get the nodes in the cluster.
+   * @return
+   */
+  def getNodes(): Set[ServiceNode]
+
+  /**
+   * Add a listener which will be notified when a new node join the cluster.
+   * @param listener
+   */
+  def registerNodeJoinListener(listener: ServiceNode => Unit): Unit
 
 Review comment:
   nit: space after `listener`.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng closed pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng closed pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r391992640
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/cluster/ClusterManager.scala
 ##########
 @@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.livy.cluster
+
+import scala.collection.mutable.Set
+
+case class ServiceNode(host: String, port: Int, UUID: String)
+
+/**
+ * Interface for cluster management.
+ */
+abstract class ClusterManager {
+  /**
+   * Register current node into the cluster. It should be invoked when the node is ready
+   * for service.
+   */
+  def register(): Unit
+
+  /**
+   * Get the nodes in the cluster.
+   * @return
+   */
+  def getNodes(): Set[ServiceNode]
 
 Review comment:
   done

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] codecov-io edited a comment on issue #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#issuecomment-576107327
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=h1) Report
   > Merging [#276](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-livy/commit/66b5833e413bc10e39e3b92b585f496444c147d4?src=pr&el=desc) will **increase** coverage by `0.11%`.
   > The diff coverage is `74.07%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-livy/pull/276/graphs/tree.svg?width=650&token=0MkVbiUFwE&height=150&src=pr)](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #276      +/-   ##
   ============================================
   + Coverage     68.14%   68.26%   +0.11%     
   - Complexity      959      980      +21     
   ============================================
     Files           104      106       +2     
     Lines          5946     5999      +53     
     Branches        899      908       +9     
   ============================================
   + Hits           4052     4095      +43     
   - Misses         1312     1313       +1     
   - Partials        582      591       +9
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...rver/src/main/scala/org/apache/livy/LivyConf.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9MaXZ5Q29uZi5zY2FsYQ==) | `96.22% <100%> (+0.09%)` | `21 <0> (ø)` | :arrow_down: |
   | [...main/scala/org/apache/livy/server/LivyServer.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvTGl2eVNlcnZlci5zY2FsYQ==) | `33.18% <33.33%> (-0.3%)` | `11 <0> (ø)` | |
   | [...scala/org/apache/livy/cluster/ClusterManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9jbHVzdGVyL0NsdXN0ZXJNYW5hZ2VyLnNjYWxh) | `50% <50%> (ø)` | `1 <1> (?)` | |
   | [...apache/livy/server/recovery/ZooKeeperManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvcmVjb3ZlcnkvWm9vS2VlcGVyTWFuYWdlci5zY2FsYQ==) | `67.24% <68.75%> (+0.57%)` | `22 <5> (+5)` | :arrow_up: |
   | [...ala/org/apache/livy/cluster/ZKClusterManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9jbHVzdGVyL1pLQ2x1c3Rlck1hbmFnZXIuc2NhbGE=) | `84% <84%> (ø)` | `14 <14> (?)` | |
   | [...ain/java/org/apache/livy/rsc/driver/RSCDriver.java](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-cnNjL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9saXZ5L3JzYy9kcml2ZXIvUlNDRHJpdmVyLmphdmE=) | `81.66% <0%> (+1.66%)` | `45% <0%> (ø)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=footer). Last update [66b5833...7b6b2a8](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng opened a new pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng opened a new pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276
 
 
   ## What changes were proposed in this pull request?
   In this patch, we check in a ClusterManager class to handle the node join/leave cluster events based on Zookeeper.
   
   Users can add listeners to monitor node join/leave.
   
   ## How was this patch tested?
   Add new unit tests and manual tests.
   
   Please review https://livy.incubator.apache.org/community/ before opening a pull request.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] yiheng closed pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
yiheng closed pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] codecov-io edited a comment on issue #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#issuecomment-576107327
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=h1) Report
   > Merging [#276](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-livy/commit/66b5833e413bc10e39e3b92b585f496444c147d4&el=desc) will **increase** coverage by `0.19%`.
   > The diff coverage is `74.07%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-livy/pull/276/graphs/tree.svg?width=650&height=150&src=pr&token=0MkVbiUFwE)](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #276      +/-   ##
   ============================================
   + Coverage     68.14%   68.34%   +0.19%     
   - Complexity      959      984      +25     
   ============================================
     Files           104      106       +2     
     Lines          5946     6005      +59     
     Branches        899      909      +10     
   ============================================
   + Hits           4052     4104      +52     
     Misses         1312     1312              
   - Partials        582      589       +7     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...main/scala/org/apache/livy/server/LivyServer.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvTGl2eVNlcnZlci5zY2FsYQ==) | `33.18% <33.33%> (-0.30%)` | `11.00 <0.00> (ø)` | |
   | [...scala/org/apache/livy/cluster/ClusterManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9jbHVzdGVyL0NsdXN0ZXJNYW5hZ2VyLnNjYWxh) | `50.00% <50.00%> (ø)` | `1.00 <1.00> (?)` | |
   | [...apache/livy/server/recovery/ZooKeeperManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvcmVjb3ZlcnkvWm9vS2VlcGVyTWFuYWdlci5zY2FsYQ==) | `67.24% <68.75%> (+0.57%)` | `22.00 <5.00> (+5.00)` | |
   | [...ala/org/apache/livy/cluster/ZKClusterManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9jbHVzdGVyL1pLQ2x1c3Rlck1hbmFnZXIuc2NhbGE=) | `84.00% <84.00%> (ø)` | `14.00 <14.00> (?)` | |
   | [...rver/src/main/scala/org/apache/livy/LivyConf.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9MaXZ5Q29uZi5zY2FsYQ==) | `96.22% <100.00%> (+0.09%)` | `21.00 <0.00> (ø)` | |
   | [...c/main/scala/org/apache/livy/repl/ReplDriver.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-cmVwbC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2xpdnkvcmVwbC9SZXBsRHJpdmVyLnNjYWxh) | `30.76% <0.00%> (-2.57%)` | `7.00% <0.00%> (ø%)` | |
   | [...va/org/apache/livy/client/http/LivyConnection.java](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-Y2xpZW50LWh0dHAvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2xpdnkvY2xpZW50L2h0dHAvTGl2eUNvbm5lY3Rpb24uamF2YQ==) | `82.27% <0.00%> (+0.22%)` | `15.00% <0.00%> (ø%)` | |
   | ... and [3 more](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=footer). Last update [66b5833...62b09aa](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] jerryshao commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
jerryshao commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r368781399
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/cluster/ZKClusterManager.scala
 ##########
 @@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.livy.cluster
+
+import java.util.UUID
+
+import scala.collection.mutable
+import scala.collection.mutable.{ArrayBuffer, Set}
+
+import org.apache.livy.rsc.RSCConf.Entry.LAUNCHER_ADDRESS
+import org.apache.livy.LivyConf.{SERVER_PORT, ZK_SERVICE_DIR}
+import org.apache.livy.LivyConf
+import org.apache.livy.Logging
+import org.apache.livy.server.recovery.ZooKeeperManager
+
+class ZKClusterManager(livyConf: LivyConf, zkManager: ZooKeeperManager)
+  extends ClusterManager with Logging {
+  private val serverIP = livyConf.get(LAUNCHER_ADDRESS)
+  require(serverIP != null, "Please config the livy.rsc.launcher.address")
+
+  private val port = livyConf.getInt(SERVER_PORT)
+  private val serviceDir = livyConf.get(ZK_SERVICE_DIR)
+
+  private val nodes = new mutable.HashSet[ServiceNode]()
+  private val nodeJoinListeners = new ArrayBuffer[ServiceNode => Unit]()
+  private val nodeLeaveListeners = new ArrayBuffer[ServiceNode => Unit]()
+
+  zkManager.getChildren(serviceDir).foreach(node => {
+    val serviceNode = zkManager.get[ServiceNode](serviceDir + "/" + node).get
+    nodes.add(serviceNode)
+  })
+
+  // Start listening
+  zkManager.watchAddNode(serviceDir, nodeAddHandler)
+  zkManager.watchRemoveNode(serviceDir, nodeRemoveHandler)
+
+  override def register(): Unit = {
+    val node = ServiceNode(serverIP, port, UUID.randomUUID().toString)
+    zkManager.createEphemeralNode(serviceDir + "/" + serverIP + ":" + port, node)
+  }
+
+  override def getNodes(): Set[ServiceNode] = {
+    nodes
+  }
+
+  override def registerNodeJoinListener(listener: ServiceNode => Unit): Unit = {
+    nodeJoinListeners.append(listener)
+  }
+
+  override def registerNodeLeaveListener(listener : ServiceNode => Unit): Unit = {
+    nodeLeaveListeners.append(listener)
+  }
+
+  private def nodeAddHandler(path: String, node: ServiceNode): Unit = {
+    logger.info("Detect new node join: " + node)
+    nodes.add(node)
+    nodeJoinListeners.foreach(_(node))
+  }
+
+  private def nodeRemoveHandler(path: String, node: ServiceNode): Unit = {
+    logger.info("Detect node leave: " + node)
+    nodes.remove(node)
+    nodeLeaveListeners.foreach(_(node))
 
 Review comment:
   Do we need to notify all the nodes when one node is joined or left?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] jerryshao commented on a change in pull request #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
jerryshao commented on a change in pull request #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#discussion_r368780351
 
 

 ##########
 File path: server/src/main/scala/org/apache/livy/LivyConf.scala
 ##########
 @@ -227,6 +243,8 @@ object LivyConf {
   val RECOVERY_ZK_STATE_STORE_KEY_PREFIX =
     Entry("livy.server.recovery.zk-state-store.key-prefix", "livy")
 
+  val ZK_SERVICE_DIR = Entry("livy.server.zk.services", "/livy/zk/services")
 
 Review comment:
   As I left the comment in the other PR, is it better to change to name to `livy.server.ha.zk-services`. We'd better to group ha related configurations together.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-livy] codecov-io commented on issue #276: [LIVY-723] Server Registration

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #276: [LIVY-723] Server Registration
URL: https://github.com/apache/incubator-livy/pull/276#issuecomment-576107327
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=h1) Report
   > Merging [#276](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-livy/commit/66b5833e413bc10e39e3b92b585f496444c147d4?src=pr&el=desc) will **increase** coverage by `0.11%`.
   > The diff coverage is `74.07%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-livy/pull/276/graphs/tree.svg?width=650&token=0MkVbiUFwE&height=150&src=pr)](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #276      +/-   ##
   ============================================
   + Coverage     68.14%   68.26%   +0.11%     
   - Complexity      959      980      +21     
   ============================================
     Files           104      106       +2     
     Lines          5946     5999      +53     
     Branches        899      908       +9     
   ============================================
   + Hits           4052     4095      +43     
   - Misses         1312     1313       +1     
   - Partials        582      591       +9
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...rver/src/main/scala/org/apache/livy/LivyConf.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9MaXZ5Q29uZi5zY2FsYQ==) | `96.22% <100%> (+0.09%)` | `21 <0> (ø)` | :arrow_down: |
   | [...main/scala/org/apache/livy/server/LivyServer.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvTGl2eVNlcnZlci5zY2FsYQ==) | `33.18% <33.33%> (-0.3%)` | `11 <0> (ø)` | |
   | [...scala/org/apache/livy/cluster/ClusterManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9jbHVzdGVyL0NsdXN0ZXJNYW5hZ2VyLnNjYWxh) | `50% <50%> (ø)` | `1 <1> (?)` | |
   | [...apache/livy/server/recovery/ZooKeeperManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9zZXJ2ZXIvcmVjb3ZlcnkvWm9vS2VlcGVyTWFuYWdlci5zY2FsYQ==) | `67.24% <68.75%> (+0.57%)` | `22 <5> (+5)` | :arrow_up: |
   | [...ala/org/apache/livy/cluster/ZKClusterManager.scala](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS9jbHVzdGVyL1pLQ2x1c3Rlck1hbmFnZXIuc2NhbGE=) | `84% <84%> (ø)` | `14 <14> (?)` | |
   | [...ain/java/org/apache/livy/rsc/driver/RSCDriver.java](https://codecov.io/gh/apache/incubator-livy/pull/276/diff?src=pr&el=tree#diff-cnNjL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9saXZ5L3JzYy9kcml2ZXIvUlNDRHJpdmVyLmphdmE=) | `81.66% <0%> (+1.66%)` | `45% <0%> (ø)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=footer). Last update [66b5833...7b6b2a8](https://codecov.io/gh/apache/incubator-livy/pull/276?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services