Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

first stage of support for dynamic invoker id assignment #2689

Merged
merged 1 commit into from Oct 17, 2017

Conversation

dgrove-oss
Copy link
Member

A prototype of one possible approach for dynamic invoker registration.
The invoker's instanceId is not provided on the command line on
startup. Instead, the invoker dynamically requests an id from
the controller during its startup via the message bus (kafka).
Once it receives an id, invoker startup continues unchanged.

On the plus side, this protocol maintains the property that invokers
are densely assigned ids starting from 0. On the minus side, it takes
an extra round of messages thus mildly delaying system startup.

An alternative approach would be to use randomly generated UUIDs as
the invoker's id and adapt the load balancer algorithm accordingly.
Using UUIDs as invoker ids might make some aspects of operations
slightly more annoying as well, as the invokers would not have
stable (or short/easy to recognize) names. This is a larger change,
but I think it is possible to implement without too much trouble.

@dgrove-oss
Copy link
Member Author

I can't add labels, but this is a WIP posted for discussion & feedback.

Copy link
Contributor

@tysonnorris tysonnorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of not requiring IDs for invokers (and controllers?), but I think it should be derived from IP (or explicitly set as it is now). Restart cases need to use the same ID afaik.


val invokerOrdinal =
if (args.length == 0) {
val registrationUUID = UUID()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If an invoker restarts on same host should it have a different UUID? Currently this will orphan messages queued for the ID just before it exited. I thought that the invoker on same host should have the same instance ID on restart. This implies that the ID is derived from the host (or cluster of hosts) somehow - we currently leverage the host IP (encoded as an int) for this purpose

abort()
}
}
val topic = s"invokerReg-${registrationUUID.asString}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requires a new topic per restart of the same invoker?

case m : InvokerIdRequestMessage => {
val response = InvokerIdResponseMessage(m.uuid, nextInvokerId)
nextInvokerId += 1
sendIdToInvoker(response)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (sending an ID back to invoker) is only required because invoker id is currently an Int, and the registration was using UUID, right? Maybe we can change it to a String to be more flexible (@dragos and I just talked about this for other reasons as well). Then there is no need to explicitly send it an ID, AND maybe it can default to the host ip (and override with an "advertise ip" config). This enforces the idea of "1 invoker per host" - not sure that breaks anything for kube.

Side note: the case we ran into was where our invoker was misconfigured and kept restarting with a different ID each restart; eventually this crashed the controller because of the volume of health topic messages that were processed into a Map during controller startup, so separately it will be good to set the health topic consumer to receive only the latest n health messages. I'll log an issue for this.

@dgrove-oss
Copy link
Member Author

Keeping the same ID across restarts is a good point. I was thinking we will need to have some mechanism for draining the topics of offline invokers, so we could just use that more frequently and not worry about reusing invoker ids across restarts. But that might not be optimal.

Perhaps the path to pursue is allowing invoker IDs to be an arbitrary alphanumeric string (anything that is legal to embed in a kafka topic name) that the invoker computes somehow without needing to communicate with the controller (by reading a command line, by computing a function on its ip address, by otherwise picking something out of its environment/filesystem, randomUUID, ...). The controller/load balancer just uses the ID (provided to it implicitly by ping messages on the health topic) to create the kafka topic invoker-ID and the loadbalancer internally maps the ID to a dense numbering of currently active invokers to support hashing incoming work to invokers.

@rabbah
Copy link
Member

rabbah commented Sep 4, 2017 via email

@ServoKvd
Copy link
Contributor

ServoKvd commented Sep 4, 2017

Hello,
(My Id is Kavitha Devara on Slack Channels)
Im in favor of making the Invoker Stateless( from Kubernetes perspective) - Am happy to see some momentum on this.
Im experimenting with the following way of obraining the InstanceId from the Controller:
"curl controller.openwhisk:8080/invokers" ---> (This should give the current list of Invokers)

and calling this API in the "Invoker", to get the next available Id, before it can bootstrap itself up with an ID.
Use the existing API to get the length of current Invokers and Instantiate the next Invoker with the next index (OR) write a new REST API in Controller.scala, to get "availableIndex" for the Invoker.

"avialable Indexes" can be a pool of Indexes/UUIDs maninatined by the Controller that may be assigned for new Invoker Instance creation.

Any thoughts on this kind of solution?

@dgrove-oss dgrove-oss force-pushed the invoker-reg branch 3 times, most recently from 731d94f to 47d2563 Compare September 7, 2017 20:08
@dgrove-oss
Copy link
Member Author

I've reworked the code based on the comments from @rabbah and @tysonnorris. There is now the ability to provide a stable invokerUUID for an invoker instance via an environment variable. The loadbalancer maintains a mapping of UUIDs to invokerIds to allow stable assignment of invoker topics across invoker restarts. The invoker can also optionally suggest the invokerId it should be assigned (to preserve all the old behavior of statically assigning the invokerIds for each instance).

@ServoKvd
Copy link
Contributor

ServoKvd commented Sep 8, 2017

Hello,
I looked at the above patch, I understand all the changes, except I was wondering, why we need to use Kfka Bus for InvokerIdRequest/Registration messages. Treating Adding/removing invokers as CRUD operations on the OpenWhisk Controller's Invenotry, I was wondering if adding a REST API as follows may be a viable design:

Invoker.scala:
import scalaj.http.{Http, HttpResponse}
// val invokerInstance = instanceId(args(0).toInt)
if(args.length ==1)
val invokerInstance = instanceId(args(0).toInt)
else {
val response: httpResponse[String] = Http("http://172.17.0.5:8000/invokerId").asString
val invokerInstance = InstanceId(response.body.toInt)
}

Note: Here, 172.17.0.5 is "openwhisk.controller" in myexample implementation - trying to figure out how to expose the DNS mappings to Invoker, so that openwhisk.controller may resolve to 172.17.0.5.

@dgrove-oss
Copy link
Member Author

I decided to use the MessagingProvider (Kafka by default) because we already are assuming the controller and invokers can communicate with each other via this service. So it seemed to add fewer constraints on how the system is deployed than having the invoker do a post to a controller REST API.

@dgrove-oss dgrove-oss force-pushed the invoker-reg branch 4 times, most recently from c49900b to 20f11b0 Compare September 11, 2017 15:49
@dgrove-oss dgrove-oss changed the title WIP on dynamic invoker id assignment first stage of support for dynamic invoker id assignment Sep 11, 2017
@dgrove-oss
Copy link
Member Author

I've squashed to a single commit and updated the commit comment to reflect the final design.

@rabbah I think this is ready for review and could have the WIP label removed.

@@ -87,6 +89,38 @@ object Invoker {

val msgProvider = SpiLoader.get[MessagingProvider]
val producer = msgProvider.getProducer(config, ec)

val invokerUUID = sys.env.getOrElse("INVOKER_ASSIGNED_UUID", UUID().toString)
val proposedInvokerId = if (args.length == 1) { args(0).toInt } else { -1 }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think either invokerUUID or proposedInvokerId must always be specified - if you allow UUID() on each restart, for example, the assigned invokerId will be incremented each time, abandoning the previous id's assigned topic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True with current simplistic code. I could document/enforce that if we think it is important to do so.

I think where this is likely to end up is that the Registrar actor would get a little more complex and interact with the rest of the load balancing subsystem. In particular, instead of blindly incrementing a counter if there is not a match and the invoker didn't propose an id, it could first look to see if there is an abandoned topic (because of an unhealthy/offline invoker) and then prefer to assign that topic to the invoker instead of creating a new topic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a check to the code to insist that either INVOKER_ASSIGNED_UUID is defined or a proposedId is provided on the command line. If neither is provided, we abort the invoker.

@rabbah rabbah added review Review for this PR has been requested and yet needs to be done. and removed wip labels Sep 12, 2017
@ghost
Copy link

ghost commented Sep 13, 2017

For Kubernetes, we would be able to leverage the hostname of the node each Invoker is running on assuming the Invoker is running as a DaemonSet. For example, in the yaml file for Invokers, you could add:

env:
- name: INVOKER_ASSIGNED_UUID
  valueFrom:
    fieldRef:
      fieldPath: spec.nodeName

I believe in Kuberentes each Kube Node has to have a unique name and so this should guarantee that the UUIDs are unique

@bbrowning
Copy link
Contributor

How does this loadbalancer bookkeeping work once we get multiple Controllers in the mix? There's some extra shared state and coordination required when assigning and remembering the UUID to Invoker id mapping?

If an Invoker dies, the only way to get an Invoker registered to that numerical Invoker id again is to use the proposedInvokerId? If I just start up a new Invoker without telling it which Invoker id to propose then it will get assigned a new topic in Kafka instead of draining from the orphaned one. So, in practice, my deployment tooling needs to keep up with Invoker ids just like it does today?

@dgrove-oss
Copy link
Member Author

For multiple controllers, the current code is assuming that every controller will see the messages on the invokerReg topic in the same order. My understanding of kafka is superficial, but I believe that is what it provides. Assuming it does, then the code in the Registrar agent should make exactly the same set of state updates in each controller process. I've done a few dozen runs with multiple controllers and it seems to work, but that isn't proof (could be dumb luck still). An alternative implementation could forgo relying on kafka and use Akka distributed data to maintain a shared counter and mapping data structure.

If the invoker dies, but restarts with the same invokerUUID it had before, it will get the same invokerId assigned to it. The main difference from what you need to do today is that although your deployment tooling still needs to assign UUIDs, they no longer have to be dense integers. For example the invokerUUID can be the Kube nodeName or the IP address of the node (if you only deploy 1 invoker instance per IP address) or some other arbitrary alpha-numeric string.

@bbrowning
Copy link
Contributor

The controllers may see the messages in the same order, but that only works if the controllers all start together. If a controller needs to be restarted or a new one added how will the state get synchronized? Or will a new controller see all messages from the invokerReg topic since the beginning of time?

And, even with one controller, what happens if that controller process gets restarted? All of the invokers also have to be restarted to get the UUID -> mapping in-sync? Otherwise the controller would start handing out invoker ids starting from 0 again, which may overlap with ids already in use?

I do think allowing dynamic invoker adding and removal is a great thing. I'm just worried about things getting out of sync during regular restart, deployment, and scaling operations.

@dgrove-oss
Copy link
Member Author

Those are good points. I hadn't thought carefully about a total controller restart. Maybe I should rework to use an external store. This is definitely cold-path code so we can afford to hit a database if we don't have a UUID=>invokerID mapping. Then the in-memory structure is just a cache of the system of record.

@ServoKvd
Copy link
Contributor

ServoKvd commented Sep 15, 2017

I think on Controller restart, it should read back the last " snapshot of Invokers status" saved before going down. The snapshot in this case can be an array of fixed Length with InvokerIds and their status( assigned/free/assigned-waiting-for-ack etc.)

@dgrove-oss
Copy link
Member Author

updated to persist the state for dynamic invoker id assignment in a Redis instance.

@dgrove-oss
Copy link
Member Author

dgrove-oss commented Sep 22, 2017

pg1 2093 🏃

Copy link
Contributor

@markusthoemmes markusthoemmes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nits on resource usage (blocking redis client, closing the consumer in the invoker) and some idiomatic nits.

}
}
val topic = s"invokerReg-${invokerName}"
val consumer = msgProvider.getConsumer(config, topic, topic)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should shut this consumer down after successfully receiving a message.

abort()
}
UUID().toString
})())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to use -1 to indicate "It's not there". Scala has the Option type to model just that.

In this particular case, you can make use of some nice helpers to make use of Option's features to fold this nicely:

val proposedInvokerId: Option[Int] = args.headOption.map(_.toInt)
val invokerName: String = sys.env.getOrElse("INVOKER_ASSIGNED_NAME", {
  if(proposedInvokerId.isDefined) {
    logger.error(this, "Must either define INVOKER_ASSIGNED_NAME or provide proposedInvokerId on command line")
    abort()
  }
  UUID().toString
})

Note: You also don't need to build an anonymous self-calling function here, a block is sufficient. You can now model the sent message to have Option as well and handle the absence of a proposal in the controller accordingly.

logger.error(this, s"Failed to get a response from controller on expected topic: ${topic}")
abort()
}
val (_, _, _, bytes) = records(0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can enforce correctness via the compiler here:

val (_, _, _, bytes) = records.headOption.getOrElse {
  logger.error(this, s"Failed to get a response from controller on expected topic: ${topic}")
  abort()
}

m.proposedId
} else {
// dynamic id assignment protocol
val priorId = redisClient.hget("controller:registar:idAssignments", m.instanceName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does seem like the redis client you're using has a blocking interface. If that's correct, this is quite dangerous, as threads will be blocked on our main dispatcher effectively reducing throughput while a registration is in progress.

It doesn't seem like we need an Actor in this case either as no state is encapsulated. Could the whole registration be handled by the message consumer then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. When I changed things to externalize the state to Redis I could have eliminated the Actor. I'll do that.

@@ -118,3 +118,21 @@ object PingMessage extends DefaultJsonProtocol {
def parse(msg: String) = Try(serdes.read(msg.parseJson))
implicit val serdes = jsonFormat(PingMessage.apply _, "name")
}

case class InvokerIdRequestMessage(instanceName: String, proposedId: Int) extends Message {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As indicated below, proposedId could be Option[Int] to model possible absence of a proposal.

@@ -87,6 +89,45 @@ object Invoker {

val msgProvider = SpiLoader.get[MessagingProvider]
val producer = msgProvider.getProducer(config, ec)

val proposedInvokerId = if (args.length == 1) { args(0).toInt } else { -1 }
val invokerName = sys.env.getOrElse("INVOKER_ASSIGNED_NAME", (() => {
Copy link
Member

@rabbah rabbah Sep 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could also read this via whisk config - we've generally avoid reading anything from sys env in the past.
it would fail if you start up the invoker without an id.

i.e., make it a required property.

@dgrove-oss
Copy link
Member Author

update to address review comments from @rabbah and @markusthoemmes.

pg1 2100 🏃

@markusthoemmes, is moving the blocking Redis calls into the handler really enough, or do I need to be finer-grained wrapping in Future { blocking { ....blocking_redis_call...} }

@markusthoemmes
Copy link
Contributor

@dgrove-oss unfortunately its not, I suggest we move to a non-blocking redis client (the current one is blocking by design as it seems).

https://github.com/etaty/rediscala seems like a great fit (its correctly licensed as well).

wskApiHost ++ Map(
dockerImageTag -> "latest",
invokerNumCore -> "4",
invokerCoreShare -> "2",
invokerContainerPolicy -> "",
invokerContainerDns -> "",
invokerContainerNetwork -> null)
invokerContainerNetwork -> null) ++
Map(invokerName -> null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could fold the invoker name into the previous map

invokerContainerNetwork -> null) ++
+      Map(invokerName -> null)

to

invokerContainerNetwork -> null,
invokerName -> null)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure. Do you think that is better? The previous map seems to be all docker related things so I was keeping logical separation. Happy to do it either way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. Makes sense.

}.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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this retry under the covers in case of contention (i.e., multiple invokers come up at once)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my understanding from reading the docs is that this just wraps the Redis INCR command and that internally Redis is single threaded, so there cannot be a need to retry. (the various INCR operations will be applied sequentially by the single Redis thread).

@dgrove-oss
Copy link
Member Author

Now simplified as suggested by @markusthoemmes. With the state for the id assignment externalized to redis, there is no need to involve the controller at all. Each invoker can self-assign ids on startup using redis for coordination.

@dgrove-oss
Copy link
Member Author

pg1 2102 🔵

logger.info(this, s"invokerReg: invoker ${invokerName} was assigned invokerId ${newId}")
newId
}
redisClient.disconnect
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Add () as this method is side-effecting. That's a common scala best-practice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized it would be better to call quit instead of disconnect, so I changed that.

In either case, the compiler doesn't like it if I put (). I think it is a property, not a method?

/Users/dgrove/code/openwhisk/openwhisk/core/invoker/src/main/scala/whisk/core/invoker/Invoker.scala:120: Boolean does not take parameters
redisClient.quit()

id
}
.getOrElse {
val invokerName = config.invokerName
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can invokerName come from an separate environment variable or arg or system property? I think it would be possible from typesafe config, but don't think so with whisk Config. In our case, since the ID is an arg we can generate it with a script, but it cannot be the IP address directly since it is not an Int, or easily translated to an Int that is "small". It would be better to use the invokerName, and leverage your approach, but we cannot do that with config.invokerName afaik without designating a name unique to each instance.

This may be more of a problem of using IndexedSeq[Int] in the controller with padding where a large value for proposedInvokerId will cause havoc due to padding of invoker registration for each id lower than the proposed.

It would be great to document this treatment of instanceid (I couldn't find any) to let devs know that using "any Int" is not acceptable; or else it would be good to have a workaround, I may be doing it wrong - we hoped to use an int derived from IP number, but the result is that, for example, the controller/invokers endpoint displays as massive quantity of invokers, all of which are offline, due to the padding. I'm not sure if it would be possible to change this type from Int to String in controllers invoker registration logic, but that would be one way to solve it, if using a referenced env var (e.g. INVOKER_NAME=${SOME_VAR}) for invoker name is not possible.

Copy link
Member

@rabbah rabbah Sep 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am reading the pr correctly the name and ordinal assigned as the id for load balancing are completely decoupled. The name could be any value/string. The increments through redis make sure you have a monotonically increasing value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My question is about whether config.invokerName has to be statically assigned as an environment variable INVOKER_NAME=MyInvokerName, or if it can be derived from either a system property or other environment variable, e.g. INVOKER_NAME=${SOME_VALUE} (where SOME_VALUE is another variable). Or, if it was possible to pass it as an arg, that would also be a way to use $SOME_VALUE, but currently it isn't.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Thanks for the clarification. My understanding is as long as you can generate a stable name (across restarts) it should no longer matter. The reason the name has to be stable is for the redis negotiation. Every new name would bump the count otherwise. So I think the naming could be generalized accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering fi a separate redis client to get the Invoker Id is an overkill. Isn't just keeping a circular buffer of " InvokerIds& Names" in the Controller sufficient to assign a new id to the Invoker. the same Circular buffer can log data from the Health stats and registration status. "Number of Invokers" per Controller can be set in Limits and higher-level scaling with HA replicated controllers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tysonnorris re config.invokerName, this is being read from the environment variable INVOKER_NAME (@rabbah had asked me to not read sys.env directly, but instead access the container's environment through the WhiskConfig object). Does this work for your scenario?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ServoKvd, I think I failed to actually post something about possible followons to this PR. As Rodric said, this PR was meant as a small step to allow us to decouple the invoker name from the kafka topic that the invoker is assigned to service. Useful for some of the kube work and to give some flexibility in deployment. In the future the decoupling could allow a fancier load balancer that could more dynamically assign invokers to topics and also support an elastic pool of invokers that could grow and shrink depending on overall system load. All details still to be worked out 😁

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgrove-oss unfortunately setting INVOKER_NAME doesn't work in our case, since the env value will be the same for each instance (when running via mesos/marathon). If it was possible to reference another env value as the value for INVOKER_NAME, it would work, e.g. INVOKER_NAME=${SOME_OTHER_ENV}, since there will be a few of these values set per instance automatically (but cannot be used to explicitly set some var unique per instance).

I think a bigger change would be required to support this for our case, like either use TypeSafe Config (instead of whisk Config), or arg flags (e.g. you can specify either id OR name as an arg), or some extension for looking up the name that may be appropriate for particular environments (e.g. in mesos we could lookup a name from some other env vars; other envs may have other ways of associating names, etc).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tysonnorris thanks; I understand the mesos/marathon constraints now. They are different than kube 😞 Would it be acceptable for me to generalize in a followup PR? I have to admit I'd like to get this one merged without more iterations so we can unblock the DaemonSet PR on the kube deploy project.

I don't think it would be that hard for me to do a followup PR to allow a combination of arg flags and environment variables to be used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgrove-oss thanks, I understand. We can work with the current requirement to assign instance id, it is just very clunky :) If you get to a followup for the other combinations I'd be happy to test it.

@@ -85,6 +86,41 @@ object Invoker {
abort()
}

val proposedInvokerId: Option[Int] = args.headOption.map(_.toInt)
val assignedInvokerId = proposedInvokerId
.map { id =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this just a foreach?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, get's getOrElsed below. Unfortunately, Option has no method to just apply a side-effect, passing the original value through (like andThen on Futures)

@dgrove-oss
Copy link
Member Author

rebased to Oct 2 master. pg4 836 🏃

@dgrove-oss
Copy link
Member Author

2 comments in favor of merging on dev list and no objections. Good to go?

Invoker ids can optionally be dynamically generated by OpenWhisk using
a deployment-specified invoker name to ensure id stability across
invoker instance restarts. The invoker name may be any unique String
(for example the ip addr of the invoker's host or the nodeName of the
kube workerNode) and _must_ be specified by the deployment by setting
the INVOKER_NAME environment variable for each invoker.
Deployments may bypass the dynamic id assignment algorithm
by optionally providing a static invoker id as a command line argument
to the invoker. If a static id is provided, it will be used
unconditionally.

The state for the invoker id assignment algorithm is externalized in a
persistent store. During startup, if a static invoker id was not
provided, the invoker interacts with the store to lookup the id
assigned to its name (causing an id to be assigned if there was no
previous assignment).

For the purposes of this PR, we use the Redis instance started for the
apigateway as the persistent store since the Redis API is a good match
for the needed operations. This function could be moved to zookeeper
in a later PR if it is desirable to minimize the assumptions about
available persistent stores.
@dgrove-oss
Copy link
Member Author

rebased yet again to master to resolve conflicts.

PG1 2150 🏃

Could we please finally merge once CI approves?

@markusthoemmes markusthoemmes merged commit fa8b462 into apache:master Oct 17, 2017
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Oct 18, 2017
Invoker ids can optionally be dynamically generated by OpenWhisk using
a deployment-specified invoker name to ensure id stability across
invoker instance restarts. The invoker name may be any unique String
(for example the ip addr of the invoker's host or the nodeName of the
kube workerNode) and _must_ be specified by the deployment by setting
the INVOKER_NAME environment variable for each invoker.
Deployments may bypass the dynamic id assignment algorithm
by optionally providing a static invoker id as a command line argument
to the invoker. If a static id is provided, it will be used
unconditionally.

The state for the invoker id assignment algorithm is externalized in a
persistent store. During startup, if a static invoker id was not
provided, the invoker interacts with the store to lookup the id
assigned to its name (causing an id to be assigned if there was no
previous assignment).

For the purposes of this PR, we use the Redis instance started for the
apigateway as the persistent store since the Redis API is a good match
for the needed operations. This function could be moved to zookeeper
in a later PR if it is desirable to minimize the assumptions about
available persistent stores.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 13, 2017
Invoker ids can optionally be dynamically generated by OpenWhisk using
a deployment-specified invoker name to ensure id stability across
invoker instance restarts. The invoker name may be any unique String
(for example the ip addr of the invoker's host or the nodeName of the
kube workerNode) and _must_ be specified by the deployment by setting
the INVOKER_NAME environment variable for each invoker.
Deployments may bypass the dynamic id assignment algorithm
by optionally providing a static invoker id as a command line argument
to the invoker. If a static id is provided, it will be used
unconditionally.

The state for the invoker id assignment algorithm is externalized in a
persistent store. During startup, if a static invoker id was not
provided, the invoker interacts with the store to lookup the id
assigned to its name (causing an id to be assigned if there was no
previous assignment).

For the purposes of this PR, we use the Redis instance started for the
apigateway as the persistent store since the Redis API is a good match
for the needed operations. This function could be moved to zookeeper
in a later PR if it is desirable to minimize the assumptions about
available persistent stores.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 13, 2017
Invoker ids can optionally be dynamically generated by OpenWhisk using
a deployment-specified invoker name to ensure id stability across
invoker instance restarts. The invoker name may be any unique String
(for example the ip addr of the invoker's host or the nodeName of the
kube workerNode) and _must_ be specified by the deployment by setting
the INVOKER_NAME environment variable for each invoker.
Deployments may bypass the dynamic id assignment algorithm
by optionally providing a static invoker id as a command line argument
to the invoker. If a static id is provided, it will be used
unconditionally.

The state for the invoker id assignment algorithm is externalized in a
persistent store. During startup, if a static invoker id was not
provided, the invoker interacts with the store to lookup the id
assigned to its name (causing an id to be assigned if there was no
previous assignment).

For the purposes of this PR, we use the Redis instance started for the
apigateway as the persistent store since the Redis API is a good match
for the needed operations. This function could be moved to zookeeper
in a later PR if it is desirable to minimize the assumptions about
available persistent stores.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 14, 2017
Invoker ids can optionally be dynamically generated by OpenWhisk using
a deployment-specified invoker name to ensure id stability across
invoker instance restarts. The invoker name may be any unique String
(for example the ip addr of the invoker's host or the nodeName of the
kube workerNode) and _must_ be specified by the deployment by setting
the INVOKER_NAME environment variable for each invoker.
Deployments may bypass the dynamic id assignment algorithm
by optionally providing a static invoker id as a command line argument
to the invoker. If a static id is provided, it will be used
unconditionally.

The state for the invoker id assignment algorithm is externalized in a
persistent store. During startup, if a static invoker id was not
provided, the invoker interacts with the store to lookup the id
assigned to its name (causing an id to be assigned if there was no
previous assignment).

For the purposes of this PR, we use the Redis instance started for the
apigateway as the persistent store since the Redis API is a good match
for the needed operations. This function could be moved to zookeeper
in a later PR if it is desirable to minimize the assumptions about
available persistent stores.
@dgrove-oss dgrove-oss deleted the invoker-reg branch November 21, 2017 14:52
BillZong pushed a commit to BillZong/openwhisk that referenced this pull request Nov 18, 2019
Invoker ids can optionally be dynamically generated by OpenWhisk using
a deployment-specified invoker name to ensure id stability across
invoker instance restarts. The invoker name may be any unique String
(for example the ip addr of the invoker's host or the nodeName of the
kube workerNode) and _must_ be specified by the deployment by setting
the INVOKER_NAME environment variable for each invoker.
Deployments may bypass the dynamic id assignment algorithm
by optionally providing a static invoker id as a command line argument
to the invoker. If a static id is provided, it will be used
unconditionally.

The state for the invoker id assignment algorithm is externalized in a
persistent store. During startup, if a static invoker id was not
provided, the invoker interacts with the store to lookup the id
assigned to its name (causing an id to be assigned if there was no
previous assignment).

For the purposes of this PR, we use the Redis instance started for the
apigateway as the persistent store since the Redis API is a good match
for the needed operations. This function could be moved to zookeeper
in a later PR if it is desirable to minimize the assumptions about
available persistent stores.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invoker review Review for this PR has been requested and yet needs to be done.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants