[GitHub] dgrove-oss commented on a change in pull request #2916: use ZooKeeper as persistent store for invokerId assignment

[GitBox <gi...@apache.org>]

dgrove-oss commented on a change in pull request #2916: use ZooKeeper as persistent store for invokerId assignment
URL: https://github.com/apache/incubator-openwhisk/pull/2916#discussion_r148795626
 
 

 ##########
 File path: core/invoker/src/main/scala/whisk/core/invoker/Invoker.scala
 ##########
 @@ -109,37 +111,47 @@ object Invoker {
         id
       }
       .getOrElse {
-        if (config.redisHostName.trim.isEmpty || config.redisHostPort.trim.isEmpty) {
-          abort(
-            s"Must provide valid Redis host and port to use dynamicId assignment (${config.redisHostName}:${config.redisHostPort})")
+        if (config.zookeeperHost.startsWith(":") || config.zookeeperHost.endsWith(":")) {
+          abort(s"Must provide valid zookeeper host and port to use dynamicId assignment (${config.zookeeperHost})")
         }
         val invokerName = config.invokerName
         if (invokerName.trim.isEmpty) {
           abort("Invoker name can't be empty to use dynamicId assignment.")
         }
-        val redisClient = new RedisClient(config.redisHostName, config.redisHostPort.toInt)
-        val assignedId = redisClient
-          .hget("controller:registar:idAssignments", invokerName)
-          .map { oldId =>
-            logger.info(this, s"invokerReg: invoker ${invokerName} was assigned its previous invokerId ${oldId}")
-            oldId.toInt
-          }
-          .getOrElse {
-            // If key not present, incr initializes to 0 before applying increment.
-            // Convert from 1-based to 0-based invokerIds by subtracting 1 from incr's result
-            val newId = redisClient
-              .incr("controller:registrar:nextInvokerId")
-              .map { id =>
-                id.toInt - 1
-              }
-              .getOrElse {
-                abort("Failed to increment invokerId")
+        logger.info(this, s"invokerReg: creating zkClient to ${config.zookeeperHost}")
+        val retryPolicy = new RetryUntilElapsed(5000, 500)
+        val zkClient = CuratorFrameworkFactory.newClient(config.zookeeperHost, retryPolicy)
+        zkClient.start()
+        zkClient.blockUntilConnected();
+        logger.info(this, "invokerReg: connected to zookeeper")
+        val myIdPath = "/invokers/idAssignment/mapping/" + invokerName
+        val assignedId =
+          if (zkClient.checkExists().forPath(myIdPath) == null) {
+            // path doesn't exist ==> no previous mapping for this invoker
+            logger.info(this, s"invokerReg: no prior assignment of id for invoker $invokerName")
+            val idCounter = new SharedCount(zkClient, "/invokers/idAssignment/counter", 0)
+            idCounter.start()
+            def assignId(): Int = {
+              val current = idCounter.getVersionedValue()
+              return if (idCounter.trySetCount(current, current.getValue() + 1)) {
+                current.getValue()
 
 Review comment:
   Purely local.  Insided the call to getVersionedValue() is where the ZooKeeper interaction happens.  The result of that call is an instance of VersionedValue, which is just a POJO with two final fields. getValue() is just a get accessor for the final field value

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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