You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2020/11/11 16:12:12 UTC
[GitHub] [openwhisk] rabbah commented on a change in pull request #5024: Reset / Overwrite invokerId for unique name in zookeeper manually
rabbah commented on a change in pull request #5024:
URL: https://github.com/apache/openwhisk/pull/5024#discussion_r521458110
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InstanceIdAssigner.scala
##########
@@ -22,50 +22,76 @@ import org.apache.curator.framework.recipes.shared.SharedCount
import org.apache.curator.retry.RetryUntilElapsed
import org.apache.openwhisk.common.Logging
+import scala.collection.JavaConverters._
+
/**
* Computes the instanceId for invoker
*
* @param connectionString zooKeeper connection string
*/
private[invoker] class InstanceIdAssigner(connectionString: String)(implicit logger: Logging) {
- def getId(name: String): Int = {
+ def setAndGetId(name: String, overwriteId: Option[Int] = None): Int = {
logger.info(this, s"invokerReg: creating zkClient to $connectionString")
val retryPolicy = new RetryUntilElapsed(5000, 500) // retry at 500ms intervals until 5 seconds have elapsed
val zkClient = CuratorFrameworkFactory.newClient(connectionString, retryPolicy)
zkClient.start()
zkClient.blockUntilConnected()
logger.info(this, "invokerReg: connected to zookeeper")
- val myIdPath = "/invokers/idAssignment/mapping/" + name
- val assignedId = Option(zkClient.checkExists().forPath(myIdPath)) match {
- case None =>
- // path doesn't exist -> no previous mapping for this invoker
- logger.info(this, s"invokerReg: no prior assignment of id for invoker $name")
- val idCounter = new SharedCount(zkClient, "/invokers/idAssignment/counter", 0)
- idCounter.start()
+ val rootPath = "/invokers/idAssignment/mapping"
+ val myIdPath = rootPath + s"/$name"
+ val assignedId = if (overwriteId.isEmpty) {
Review comment:
scalaism here - rather than check if the option is empty, you can do
```scala
overwriteId.map(id => the not empty case ).getOrElse( the empty case )
```
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InstanceIdAssigner.scala
##########
@@ -22,50 +22,76 @@ import org.apache.curator.framework.recipes.shared.SharedCount
import org.apache.curator.retry.RetryUntilElapsed
import org.apache.openwhisk.common.Logging
+import scala.collection.JavaConverters._
+
/**
* Computes the instanceId for invoker
*
* @param connectionString zooKeeper connection string
*/
private[invoker] class InstanceIdAssigner(connectionString: String)(implicit logger: Logging) {
- def getId(name: String): Int = {
+ def setAndGetId(name: String, overwriteId: Option[Int] = None): Int = {
logger.info(this, s"invokerReg: creating zkClient to $connectionString")
val retryPolicy = new RetryUntilElapsed(5000, 500) // retry at 500ms intervals until 5 seconds have elapsed
val zkClient = CuratorFrameworkFactory.newClient(connectionString, retryPolicy)
zkClient.start()
zkClient.blockUntilConnected()
logger.info(this, "invokerReg: connected to zookeeper")
- val myIdPath = "/invokers/idAssignment/mapping/" + name
- val assignedId = Option(zkClient.checkExists().forPath(myIdPath)) match {
- case None =>
- // path doesn't exist -> no previous mapping for this invoker
- logger.info(this, s"invokerReg: no prior assignment of id for invoker $name")
- val idCounter = new SharedCount(zkClient, "/invokers/idAssignment/counter", 0)
- idCounter.start()
+ val rootPath = "/invokers/idAssignment/mapping"
+ val myIdPath = rootPath + s"/$name"
+ val assignedId = if (overwriteId.isEmpty) {
+ Option(zkClient.checkExists().forPath(myIdPath)) match {
+ case None =>
+ // path doesn't exist -> no previous mapping for this invoker
+ logger.info(this, s"invokerReg: no prior assignment of id for invoker $name")
+ val idCounter = new SharedCount(zkClient, "/invokers/idAssignment/counter", 0)
+ idCounter.start()
- def assignId(): Int = {
- val current = idCounter.getVersionedValue()
- if (idCounter.trySetCount(current, current.getValue() + 1)) {
- current.getValue()
- } else {
- assignId()
+ def assignId(): Int = {
+ val current = idCounter.getVersionedValue()
+ if (idCounter.trySetCount(current, current.getValue() + 1)) {
+ current.getValue()
+ } else {
+ assignId()
+ }
}
- }
- val newId = assignId()
- idCounter.close()
- zkClient.create().creatingParentContainersIfNeeded().forPath(myIdPath, BigInt(newId).toByteArray)
- logger.info(this, s"invokerReg: invoker $name was assigned invokerId $newId")
- newId
+ val newId = assignId()
+ idCounter.close()
+ zkClient.create().creatingParentContainersIfNeeded().forPath(myIdPath, BigInt(newId).toByteArray)
+ logger.info(this, s"invokerReg: invoker $name was assigned invokerId $newId")
+ newId
+
+ case Some(_) =>
+ // path already exists -> there is a previous mapping for this invoker we should use
+ val rawOldId = zkClient.getData().forPath(myIdPath)
+ val oldId = BigInt(rawOldId).intValue
+ logger.info(this, s"invokerReg: invoker $name was assigned its previous invokerId $oldId")
+ oldId
+ }
+ } else {
+ val newId = overwriteId.get
+
+ //check if the invokerId already exists for another unique name
+ val instanceIdExists = zkClient
+ .getChildren()
+ .forPath(rootPath)
+ .asScala
+ .map(uniqueName => {
+ val idPath = rootPath + s"/$uniqueName"
Review comment:
```suggestion
val idPath = s"$rootPath/$uniqueName"
```
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InstanceIdAssigner.scala
##########
@@ -22,50 +22,76 @@ import org.apache.curator.framework.recipes.shared.SharedCount
import org.apache.curator.retry.RetryUntilElapsed
import org.apache.openwhisk.common.Logging
+import scala.collection.JavaConverters._
+
/**
* Computes the instanceId for invoker
*
* @param connectionString zooKeeper connection string
*/
private[invoker] class InstanceIdAssigner(connectionString: String)(implicit logger: Logging) {
- def getId(name: String): Int = {
+ def setAndGetId(name: String, overwriteId: Option[Int] = None): Int = {
logger.info(this, s"invokerReg: creating zkClient to $connectionString")
val retryPolicy = new RetryUntilElapsed(5000, 500) // retry at 500ms intervals until 5 seconds have elapsed
val zkClient = CuratorFrameworkFactory.newClient(connectionString, retryPolicy)
zkClient.start()
zkClient.blockUntilConnected()
logger.info(this, "invokerReg: connected to zookeeper")
- val myIdPath = "/invokers/idAssignment/mapping/" + name
- val assignedId = Option(zkClient.checkExists().forPath(myIdPath)) match {
- case None =>
- // path doesn't exist -> no previous mapping for this invoker
- logger.info(this, s"invokerReg: no prior assignment of id for invoker $name")
- val idCounter = new SharedCount(zkClient, "/invokers/idAssignment/counter", 0)
- idCounter.start()
+ val rootPath = "/invokers/idAssignment/mapping"
+ val myIdPath = rootPath + s"/$name"
Review comment:
```suggestion
val myIdPath = s"$rootPath/$name"
```
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InstanceIdAssigner.scala
##########
@@ -22,50 +22,76 @@ import org.apache.curator.framework.recipes.shared.SharedCount
import org.apache.curator.retry.RetryUntilElapsed
import org.apache.openwhisk.common.Logging
+import scala.collection.JavaConverters._
+
/**
* Computes the instanceId for invoker
*
* @param connectionString zooKeeper connection string
*/
private[invoker] class InstanceIdAssigner(connectionString: String)(implicit logger: Logging) {
- def getId(name: String): Int = {
+ def setAndGetId(name: String, overwriteId: Option[Int] = None): Int = {
logger.info(this, s"invokerReg: creating zkClient to $connectionString")
val retryPolicy = new RetryUntilElapsed(5000, 500) // retry at 500ms intervals until 5 seconds have elapsed
val zkClient = CuratorFrameworkFactory.newClient(connectionString, retryPolicy)
zkClient.start()
zkClient.blockUntilConnected()
logger.info(this, "invokerReg: connected to zookeeper")
- val myIdPath = "/invokers/idAssignment/mapping/" + name
- val assignedId = Option(zkClient.checkExists().forPath(myIdPath)) match {
- case None =>
- // path doesn't exist -> no previous mapping for this invoker
- logger.info(this, s"invokerReg: no prior assignment of id for invoker $name")
- val idCounter = new SharedCount(zkClient, "/invokers/idAssignment/counter", 0)
- idCounter.start()
+ val rootPath = "/invokers/idAssignment/mapping"
+ val myIdPath = rootPath + s"/$name"
+ val assignedId = if (overwriteId.isEmpty) {
+ Option(zkClient.checkExists().forPath(myIdPath)) match {
+ case None =>
+ // path doesn't exist -> no previous mapping for this invoker
+ logger.info(this, s"invokerReg: no prior assignment of id for invoker $name")
+ val idCounter = new SharedCount(zkClient, "/invokers/idAssignment/counter", 0)
+ idCounter.start()
- def assignId(): Int = {
- val current = idCounter.getVersionedValue()
- if (idCounter.trySetCount(current, current.getValue() + 1)) {
- current.getValue()
- } else {
- assignId()
+ def assignId(): Int = {
+ val current = idCounter.getVersionedValue()
+ if (idCounter.trySetCount(current, current.getValue() + 1)) {
+ current.getValue()
+ } else {
+ assignId()
Review comment:
it's "possible" this can recurse forever, should this give up after N trials?
----------------------------------------------------------------
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