[GitHub] [openwhisk] style95 commented on issue #4626: Allow limiting DB bloat by excluding response from Activation record in some cases
style95 commented on issue #4626: Allow limiting DB bloat by excluding response from Activation record in some cases URL: https://github.com/apache/openwhisk/issues/4626#issuecomment-532503097 @sven-lange-last All makes sense to me. We also introduced a flag(`volatile`) to let users decide whether to store their activations or not when we were using CouchDB as an activation store. At that time, it was an optional flag so if a user does not explicitly set the flag, all activations were stored. Since it was disabled by default, no one tried to use it. We had to urge heavy users to use the flag. Since the main problem is the scalability of the system, users are supposed not to consider the system issue. They always wanted to see their results no matter of the environment(dev/production). Even though we enable the feature by default, we can not stop users disabling the flag(store activations) all the time. We stored failed activations even though the flag is enabled for better debugging, but anyway users wanted to see their successful activation results sometimes. Not only the activation results but they also wanted to see metadata such as `initTime`, `waitTime`, etc. (I think if we want to reduce the size of activaitons, it could be one option to store metadata and just skip results.) So we dropped `volatile` flag, stopped using `CouchDbActivationStore` and introduced `ElasticSearchActivationStore`. Regarding reducing the number of activations, I think there are also two aspects. 1. Reducing the number of activations/second 2. Reducing the number of stored activations. With CouchDB, we observed issues in both cases. (Even with 10GB data, CouchDB sometimes started dragging.) Regarding number 1, I think even though we reduce the amount, there would be a limitation as the size of cluster grows. We anyway need to handle(store) some portion of activations and if the cluster scales, anyway this portion would be beyond the CouchDB can handle. I still agree that this would be a good option no matter which datastores are used, but anyway we need to secure some level of scalability at some point. Regarding number 2, most of the users query relatively recent activations. They tend to query activations within 1 month ~ 3 months. It would not be cost-effective to store all activation data during 1 year ~ 2 years. So we decided to keep relatively "recent", but "all" activations. (I think many other OW operators already took a similar approach.) So I think our datastore should have enough scalability to handle some level of requests/s and it should have the ability to take care of "cold" data which is rarely accessed. ElasticSearch is a great option for this. With regard to log collection, I have been curious whether there "is" a case where one function generates 10MB logs. I am not sure it is realistic. As the granularity level of invocation is small in the serverless world, I think the size of the logs should be small as well. Currently, logs are collected asynchronously aside from activation storing even though logs are also included in activation data. I think this is because the log size can be up to 10MB. If we can limit the size of the maximum logs to relatively small one such as 1 MB or 512KB,(I think this is also quite big enough) we can just store logs with activations with one request. Storing logs with ELK is great but if we store them along with activations in ElasticSearch, it would give another value to users. Users can query data using logs in conjunction with metadata. (e.g: They might want to see logs whose `waitTime` is bigger than 1s.) 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk-devtools] pritidesai merged pull request #290: Automate the JVM pre-caching of classes for a given profile
pritidesai merged pull request #290: Automate the JVM pre-caching of classes for a given profile URL: https://github.com/apache/openwhisk-devtools/pull/290 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4624: Combines active ack and slot release when both are available.
rabbah commented on a change in pull request #4624: Combines active ack and
slot release when both are available.
URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325353109
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -64,43 +64,34 @@ case class ActivationMessage(override val transid:
TransactionId,
def causedBySequence: Boolean = cause.isDefined
}
-object ActivationMessage extends DefaultJsonProtocol {
-
- def parse(msg: String) = Try(serdes.read(msg.parseJson))
-
- private implicit val fqnSerdes = FullyQualifiedEntityName.serdes
- implicit val serdes = jsonFormat11(ActivationMessage.apply)
-}
-
/**
* Message that is sent from the invoker to the controller after action is
completed or after slot is free again for
* new actions.
*/
abstract class AcknowledegmentMessage(private val tid: TransactionId) extends
Message {
override val transid: TransactionId = tid
- override def serialize: String = {
-AcknowledegmentMessage.serdes.write(this).compactPrint
- }
+ override def serialize: String =
AcknowledegmentMessage.serdes.write(this).compactPrint
+ def toJson: JsValue
}
/**
- * This message is sent from the invoker to the controller, after the slot of
an invoker that has been used by the
- * current action, is free again (after log collection)
+ * This message is sent from an invoker to the controller, after the resource
slot in the invoke, used by the
+ * corresponding activation, is free again (i.e., after log collection). In
some cases, the activation result is
+ * ready and the slot is freed at the same time. In such cases, the completion
message carries the result as well.
+ * This is reflected by the of a Right() `response` and the param `result` is
set to true.
+ * In some cases the `result` is true but the response is Left() if the
message was too large for the message bus.
*/
case class CompletionMessage(override val transid: TransactionId,
- activationId: ActivationId,
- isSystemError: Boolean,
+ response: Either[ActivationId, WhiskActivation],
+ result: Boolean, // true iff the message is a
combined active ack and slot released
Review comment:
the `ack` method will have to change to support this. This is the type of
the ack method:
```
type ActiveAck = (TransactionId, WhiskActivation, Boolean,
ControllerInstanceId, UUID, Boolean) => Future[Any]
```
The last parameter in this function indicates if the `slot` is free or not.
So it is ambiguous to know inside the method if the result is combined or not
from this signature alone since every call already receives a WhiskActivation.
So I'm OK with making the change to the message types (2 to 3) but note that
the change will cause further refactoring in the invoker code.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] selfxp commented on a change in pull request #4584: OpenWhisk User Events
selfxp commented on a change in pull request #4584: OpenWhisk User Events URL: https://github.com/apache/openwhisk/pull/4584#discussion_r325345830 ## File path: core/monitoring/user-events/README.md ## @@ -0,0 +1,55 @@ + + +#  + +# OpenWhisk User Events + +This service connects to `events` topic and publishes the events to various services like Prometheus, Datadog etc via Kamon. Refer to [user specific metrics][1] on how to enable them. Review comment: yes good idea, adding a reference 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] duynguyen commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images
duynguyen commented on a change in pull request #4503: Add optional config for
appending custom registry to user provided images
URL: https://github.com/apache/openwhisk/pull/4503#discussion_r325299534
##
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala
##
@@ -60,10 +62,14 @@ class DockerContainerFactory(instance: InvokerInstanceId,
userProvidedImage: Boolean,
memory: ByteSize,
cpuShares: Int)(implicit config: WhiskConfig,
logging: Logging): Future[Container] = {
+val registryConfig =
+ ContainerFactory.resolveRegisterConfig(userProvidedImage,
runtimesRegistryConfig, userImagesRegistryConfig)
+val image =
+ if (userProvidedImage && registryConfig.url.isEmpty) Left(actionImage)
+ else Right(actionImage.localImageName(registryConfig.url))
Review comment:
I guess you meant it to be:
```
val registryConfig =
ContainerFactory.resolveRegisterConfig(userProvidedImage,
runtimesRegistryConfig, userImagesRegistryConfig)
val imageRef = actionImage.localImageName(registryConfig.url)
val image = if (userProvidedImage) Left(actionImage) else Right(imageRef)
```
(`actionImage` in Left, not `imageRef`)
However the condition is not the same as before anymore. In the case of
`userProvidedImage==true`, the raw `imageName` object is passed into
DockerContainer, which will pull the "public image name" (without custom
blackbox registry), see
[DockerContainer.scala#L105](https://github.com/apache/openwhisk/blob/master/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainer.scala#L105).
Hence I added a condition for registryConfig being empty or not.
In general this flow does not fit well into the existing flow of
DockerContainerFactory. I am working on an enhancement to get rid of
`publicImageName`, make `localImageName` (to be renamed) a single image name
resolver, and apply it throughout the code base. I will push that change for
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] dgrove-oss commented on issue #4629: remove spurious execute permission from ansible group_var files
dgrove-oss commented on issue #4629: remove spurious execute permission from ansible group_var files URL: https://github.com/apache/openwhisk/pull/4629#issuecomment-532309126 Thanks @sven-lange-last ; please merge at your convenience. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4584: OpenWhisk User Events
sven-lange-last commented on a change in pull request #4584: OpenWhisk User Events URL: https://github.com/apache/openwhisk/pull/4584#discussion_r325277569 ## File path: core/monitoring/user-events/README.md ## @@ -0,0 +1,55 @@ + + +#  + +# OpenWhisk User Events + +This service connects to `events` topic and publishes the events to various services like Prometheus, Datadog etc via Kamon. Refer to [user specific metrics][1] on how to enable them. Review comment: I suggest to extend https://github.com/apache/openwhisk/blob/master/docs/metrics.md#user-specific-metrics to point to the metrics emitter provided by this PR. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] selfxp commented on a change in pull request #4584: OpenWhisk User Events
selfxp commented on a change in pull request #4584: OpenWhisk User Events
URL: https://github.com/apache/openwhisk/pull/4584#discussion_r324002293
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -223,6 +256,15 @@ object Activation extends DefaultJsonProtocol {
"memory",
"causedBy")
Review comment:
I'll create an issue for this
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images
sven-lange-last commented on a change in pull request #4503: Add optional
config for appending custom registry to user provided images
URL: https://github.com/apache/openwhisk/pull/4503#discussion_r325263353
##
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala
##
@@ -60,10 +62,14 @@ class DockerContainerFactory(instance: InvokerInstanceId,
userProvidedImage: Boolean,
memory: ByteSize,
cpuShares: Int)(implicit config: WhiskConfig,
logging: Logging): Future[Container] = {
+val registryConfig =
+ ContainerFactory.resolveRegisterConfig(userProvidedImage,
runtimesRegistryConfig, userImagesRegistryConfig)
+val image =
+ if (userProvidedImage && registryConfig.url.isEmpty) Left(actionImage)
+ else Right(actionImage.localImageName(registryConfig.url))
Review comment:
Would this lead to the same result - as done in the Kube container factory
below?
```
val registryConfig =
ContainerFactory.resolveRegisterConfig(userProvidedImage,
runtimesRegistryConfig, userImagesRegistryConfig)
val imageRef = ContainerFactory.resolveImage(actionImage, registryConfig)
val image = if (userProvidedImage) Left(imageRef) else Right(imageRef)
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images
sven-lange-last commented on a change in pull request #4503: Add optional
config for appending custom registry to user provided images
URL: https://github.com/apache/openwhisk/pull/4503#discussion_r325259405
##
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala
##
@@ -60,10 +62,14 @@ class DockerContainerFactory(instance: InvokerInstanceId,
userProvidedImage: Boolean,
memory: ByteSize,
cpuShares: Int)(implicit config: WhiskConfig,
logging: Logging): Future[Container] = {
+val registryConfig =
+ ContainerFactory.resolveRegisterConfig(userProvidedImage,
runtimesRegistryConfig, userImagesRegistryConfig)
+val image =
+ if (userProvidedImage && registryConfig.url.isEmpty) Left(actionImage)
+ else Right(actionImage.localImageName(registryConfig.url))
Review comment:
I have the expression that this code can be streamlined as follows to
simplify the condition in line 68 (`(userProvidedImage &&
registryConfig.url.isEmpty)`):
```
val registryConfig =
ContainerFactory.resolveRegisterConfig(userProvidedImage,
runtimesRegistryConfig, userImagesRegistryConfig)
val imageRef = actionImage.localImageName(registryConfig.url)
val image = if (userProvidedImage) Left(imageRef) else Right(imageRef)
```
My understanding is that `actionImage.localImageName()` would just leave out
the registry part if no registry urls are specified.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] duynguyen commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images
duynguyen commented on a change in pull request #4503: Add optional config for
appending custom registry to user provided images
URL: https://github.com/apache/openwhisk/pull/4503#discussion_r325250701
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerFactory.scala
##
@@ -111,6 +111,20 @@ object ContainerFactory {
/** include the instance name, if specified and strip invalid chars before
attempting to use them in the container name */
def containerNamePrefix(instanceId: InvokerInstanceId): String =
s"wsk${instanceId.uniqueName.getOrElse("")}${instanceId.toInt}".filter(isAllowed)
+
+ def resolveRegisterConfig(userProvidedImage: Boolean,
+runtimesRegistryConfig: RuntimesRegistryConfig,
+userImagesRegistryConfig: RuntimesRegistryConfig):
RuntimesRegistryConfig = {
Review comment:
yes that makes sense. In fact that was a typo with `resolveRegisterConfig `
...
thanks for pointing out.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images
sven-lange-last commented on a change in pull request #4503: Add optional
config for appending custom registry to user provided images
URL: https://github.com/apache/openwhisk/pull/4503#discussion_r325249961
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerFactory.scala
##
@@ -111,6 +111,20 @@ object ContainerFactory {
/** include the instance name, if specified and strip invalid chars before
attempting to use them in the container name */
def containerNamePrefix(instanceId: InvokerInstanceId): String =
s"wsk${instanceId.uniqueName.getOrElse("")}${instanceId.toInt}".filter(isAllowed)
+
+ def resolveRegisterConfig(userProvidedImage: Boolean,
+runtimesRegistryConfig: RuntimesRegistryConfig,
+userImagesRegistryConfig: RuntimesRegistryConfig):
RuntimesRegistryConfig = {
Review comment:
Wouldn't it make sense to name this function `resolveRegistryConfig()`?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4624: Combines active ack and slot release when both are available.
sven-lange-last commented on a change in pull request #4624: Combines active
ack and slot release when both are available.
URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325238811
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -112,49 +119,51 @@ object CompletionMessage extends DefaultJsonProtocol {
case class ResultMessage(override val transid: TransactionId, response:
Either[ActivationId, WhiskActivation])
extends AcknowledegmentMessage(transid) {
- override def toString = {
-response.fold(l => l, r => r.activationId).asString
- }
+ override def toJson: JsValue = ResultMessage.serdes.write(this)
+ override def toString = response.fold(identity, _.activationId).asString
}
-object ResultMessage extends DefaultJsonProtocol {
- implicit def eitherResponse =
-new JsonFormat[Either[ActivationId, WhiskActivation]] {
- def write(either: Either[ActivationId, WhiskActivation]) = either match {
-case Right(a) => a.toJson
-case Left(b) => b.toJson
- }
+object ActivationMessage extends DefaultJsonProtocol {
+ def parse(msg: String) = Try(serdes.read(msg.parseJson))
- def read(value: JsValue) = value match {
-// per the ActivationId's serializer, it is guaranteed to be a String
even if it only consists of digits
-case _: JsString => Left(value.convertTo[ActivationId])
-case _: JsObject => Right(value.convertTo[WhiskActivation])
-case _ => deserializationError("could not read
ResultMessage")
- }
-}
+ private implicit val fqnSerdes = FullyQualifiedEntityName.serdes
+ implicit val serdes = jsonFormat11(ActivationMessage.apply)
+}
Review comment:
Yes, I'm talking about the companion objects. My concern was that developers
may miss to update the JSON (de-)serialization of case classes if case classes
and their companion objects are at different locations of the source file.
If the compiler would notice mismatches, I have no problem with what you did.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4624: Combines active ack and slot release when both are available.
rabbah commented on a change in pull request #4624: Combines active ack and
slot release when both are available.
URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325227333
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -112,49 +119,51 @@ object CompletionMessage extends DefaultJsonProtocol {
case class ResultMessage(override val transid: TransactionId, response:
Either[ActivationId, WhiskActivation])
extends AcknowledegmentMessage(transid) {
- override def toString = {
-response.fold(l => l, r => r.activationId).asString
- }
+ override def toJson: JsValue = ResultMessage.serdes.write(this)
+ override def toString = response.fold(identity, _.activationId).asString
}
-object ResultMessage extends DefaultJsonProtocol {
- implicit def eitherResponse =
-new JsonFormat[Either[ActivationId, WhiskActivation]] {
- def write(either: Either[ActivationId, WhiskActivation]) = either match {
-case Right(a) => a.toJson
-case Left(b) => b.toJson
- }
+object ActivationMessage extends DefaultJsonProtocol {
+ def parse(msg: String) = Try(serdes.read(msg.parseJson))
- def read(value: JsValue) = value match {
-// per the ActivationId's serializer, it is guaranteed to be a String
even if it only consists of digits
-case _: JsString => Left(value.convertTo[ActivationId])
-case _: JsObject => Right(value.convertTo[WhiskActivation])
-case _ => deserializationError("could not read
ResultMessage")
- }
-}
+ private implicit val fqnSerdes = FullyQualifiedEntityName.serdes
+ implicit val serdes = jsonFormat11(ActivationMessage.apply)
+}
Review comment:
Are you referring to the Objects? I found the interleaving problematic and
found unnecessary duplication (which I removed in this patch). I suspect the
duplication was caused by not actually seeing that the code was very similar
because of intervening functions. Now the objects are much more compact. So I
like the current flow better of cases classes, then the singletons.
If you change the type of the activation message into something incompatible
the code will not compile. `jsonFormatN` is an auto generated serdes and so if
the compiler doesn't know what to do, it will complain.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] codecov-io commented on issue #4629: remove spurious execute permission from ansible group_var files
codecov-io commented on issue #4629: remove spurious execute permission from ansible group_var files URL: https://github.com/apache/openwhisk/pull/4629#issuecomment-532255202 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4629?src=pr=h1) Report > Merging [#4629](https://codecov.io/gh/apache/openwhisk/pull/4629?src=pr=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/2036548e62dbf959d91c2328e86318bd7cfa656f?src=pr=desc) will **decrease** coverage by `7%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/openwhisk/pull/4629?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4629 +/- ## == - Coverage 84.44% 77.43% -7.01% == Files 183 183 Lines8305 8305 Branches 574 574 == - Hits 7013 6431 -582 - Misses 1292 1874 +582 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4629?src=pr=tree) | Coverage Δ | | |---|---|---| | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/4629/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/4629/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/openwhisk/pull/4629/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.89%)` | :arrow_down: | | [...tabase/cosmosdb/cache/CacheInvalidatorConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4629/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3JDb25maWcuc2NhbGE=) | `0% <0%> (-94.74%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/openwhisk/pull/4629/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | [...e/database/cosmosdb/cache/ChangeFeedListener.scala](https://codecov.io/gh/apache/openwhisk/pull/4629/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NoYW5nZUZlZWRMaXN0ZW5lci5zY2FsYQ==) | `0% <0%> (-86.67%)` | :arrow_down: | | [...e/database/cosmosdb/cache/KafkaEventProducer.scala](https://codecov.io/gh/apache/openwhisk/pull/4629/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0thZmthRXZlbnRQcm9kdWNlci5zY2FsYQ==) | `0% <0%> (-76.48%)` | :arrow_down: | | [...whisk/core/database/cosmosdb/CosmosDBSupport.scala](https://codecov.io/gh/apache/openwhisk/pull/4629/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJTdXBwb3J0LnNjYWxh) | `0% <0%> (-74.08%)` | :arrow_down: | | [...a/org/apache/openwhisk/common/ExecutorCloser.scala](https://codecov.io/gh/apache/openwhisk/pull/4629/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvbW1vbi9FeGVjdXRvckNsb3Nlci5zY2FsYQ==) | `0% <0%> (-66.67%)` | :arrow_down: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4629/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `4% <0%> (-52%)` | :arrow_down: | | ... and [30 more](https://codecov.io/gh/apache/openwhisk/pull/4629/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4629?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4624: Combines active ack and slot release when both are available.
sven-lange-last commented on a change in pull request #4624: Combines active
ack and slot release when both are available.
URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325204110
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -64,43 +64,34 @@ case class ActivationMessage(override val transid:
TransactionId,
def causedBySequence: Boolean = cause.isDefined
}
-object ActivationMessage extends DefaultJsonProtocol {
-
- def parse(msg: String) = Try(serdes.read(msg.parseJson))
-
- private implicit val fqnSerdes = FullyQualifiedEntityName.serdes
- implicit val serdes = jsonFormat11(ActivationMessage.apply)
-}
-
/**
* Message that is sent from the invoker to the controller after action is
completed or after slot is free again for
* new actions.
*/
abstract class AcknowledegmentMessage(private val tid: TransactionId) extends
Message {
override val transid: TransactionId = tid
- override def serialize: String = {
-AcknowledegmentMessage.serdes.write(this).compactPrint
- }
+ override def serialize: String =
AcknowledegmentMessage.serdes.write(this).compactPrint
+ def toJson: JsValue
}
/**
- * This message is sent from the invoker to the controller, after the slot of
an invoker that has been used by the
- * current action, is free again (after log collection)
+ * This message is sent from an invoker to the controller, after the resource
slot in the invoke, used by the
+ * corresponding activation, is free again (i.e., after log collection). In
some cases, the activation result is
+ * ready and the slot is freed at the same time. In such cases, the completion
message carries the result as well.
+ * This is reflected by the of a Right() `response` and the param `result` is
set to true.
+ * In some cases the `result` is true but the response is Left() if the
message was too large for the message bus.
*/
case class CompletionMessage(override val transid: TransactionId,
- activationId: ActivationId,
- isSystemError: Boolean,
+ response: Either[ActivationId, WhiskActivation],
+ result: Boolean, // true iff the message is a
combined active ack and slot released
Review comment:
Based on the discussion above, I would say that three different messages
sound like the best option. From a data modelling perspective, it's asymmetric
that a `Right( WhiskActivation)` "should not happen" in the `CompletionMessage`
if `result = false`.
With the split of the active ack into result / completion ack, we somewhat
started a renaming. So when introducing a concept with three messages, I
suggest to further follow the new naming and use something like
`ResultMessage`, `CompletionMessage` and `CombinedResultCompletionMessage`. To
preserve consistency, I would update all comments touched by this PR and
replace "active ack" with the new terms.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4624: Combines active ack and slot release when both are available.
sven-lange-last commented on a change in pull request #4624: Combines active
ack and slot release when both are available.
URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325194847
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -112,49 +119,51 @@ object CompletionMessage extends DefaultJsonProtocol {
case class ResultMessage(override val transid: TransactionId, response:
Either[ActivationId, WhiskActivation])
extends AcknowledegmentMessage(transid) {
- override def toString = {
-response.fold(l => l, r => r.activationId).asString
- }
+ override def toJson: JsValue = ResultMessage.serdes.write(this)
+ override def toString = response.fold(identity, _.activationId).asString
}
-object ResultMessage extends DefaultJsonProtocol {
- implicit def eitherResponse =
-new JsonFormat[Either[ActivationId, WhiskActivation]] {
- def write(either: Either[ActivationId, WhiskActivation]) = either match {
-case Right(a) => a.toJson
-case Left(b) => b.toJson
- }
+object ActivationMessage extends DefaultJsonProtocol {
+ def parse(msg: String) = Try(serdes.read(msg.parseJson))
- def read(value: JsValue) = value match {
-// per the ActivationId's serializer, it is guaranteed to be a String
even if it only consists of digits
-case _: JsString => Left(value.convertTo[ActivationId])
-case _: JsObject => Right(value.convertTo[WhiskActivation])
-case _ => deserializationError("could not read
ResultMessage")
- }
-}
+ private implicit val fqnSerdes = FullyQualifiedEntityName.serdes
+ implicit val serdes = jsonFormat11(ActivationMessage.apply)
+}
Review comment:
As far as I can see in the diff, you moved the JSON (de-)serialization of
`ActivationMessage` down in the source file. I would suggest to keep these JSON
(de-)serialization details in close proximity to the case class. I guess
`jsonFormat11(ActivationMessage.apply)` won't fail during compile if you add
more fields to the case class?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4624: Combines active ack and slot release when both are available.
sven-lange-last commented on a change in pull request #4624: Combines active
ack and slot release when both are available.
URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325189762
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -64,43 +64,34 @@ case class ActivationMessage(override val transid:
TransactionId,
def causedBySequence: Boolean = cause.isDefined
}
-object ActivationMessage extends DefaultJsonProtocol {
-
- def parse(msg: String) = Try(serdes.read(msg.parseJson))
-
- private implicit val fqnSerdes = FullyQualifiedEntityName.serdes
- implicit val serdes = jsonFormat11(ActivationMessage.apply)
-}
-
/**
* Message that is sent from the invoker to the controller after action is
completed or after slot is free again for
* new actions.
*/
abstract class AcknowledegmentMessage(private val tid: TransactionId) extends
Message {
override val transid: TransactionId = tid
- override def serialize: String = {
-AcknowledegmentMessage.serdes.write(this).compactPrint
- }
+ override def serialize: String =
AcknowledegmentMessage.serdes.write(this).compactPrint
+ def toJson: JsValue
}
/**
- * This message is sent from the invoker to the controller, after the slot of
an invoker that has been used by the
- * current action, is free again (after log collection)
+ * This message is sent from an invoker to the controller, after the resource
slot in the invoke, used by the
+ * corresponding activation, is free again (i.e., after log collection). In
some cases, the activation result is
+ * ready and the slot is freed at the same time. In such cases, the completion
message carries the result as well.
+ * This is reflected by the of a Right() `response` and the param `result` is
set to true.
+ * In some cases the `result` is true but the response is Left() if the
message was too large for the message bus.
*/
case class CompletionMessage(override val transid: TransactionId,
- activationId: ActivationId,
- isSystemError: Boolean,
+ response: Either[ActivationId, WhiskActivation],
+ result: Boolean, // true iff the message is a
combined active ack and slot released
Review comment:
@rabbah I do not have the requirement to support rolling updates at the
moment. From my side, we can introduce incompatible Kafka message changes
requiring that all controllers and invokers need to be updated in parallel.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] codecov-io edited a comment on issue #4627: Add descriptions how to update, remove or rename runtime kinds and language families
codecov-io edited a comment on issue #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#issuecomment-532154575 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr=h1) Report > Merging [#4627](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/2036548e62dbf959d91c2328e86318bd7cfa656f?src=pr=desc) will **decrease** coverage by `5.65%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4627 +/- ## == - Coverage 84.44% 78.78% -5.66% == Files 183 183 Lines8305 8305 Branches 574 574 == - Hits 7013 6543 -470 - Misses 1292 1762 +470 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr=tree) | Coverage Δ | | |---|---|---| | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.89%)` | :arrow_down: | | [...tabase/cosmosdb/cache/CacheInvalidatorConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3JDb25maWcuc2NhbGE=) | `0% <0%> (-94.74%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | [...e/database/cosmosdb/cache/ChangeFeedListener.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NoYW5nZUZlZWRMaXN0ZW5lci5zY2FsYQ==) | `0% <0%> (-86.67%)` | :arrow_down: | | [...e/database/cosmosdb/cache/KafkaEventProducer.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0thZmthRXZlbnRQcm9kdWNlci5zY2FsYQ==) | `0% <0%> (-76.48%)` | :arrow_down: | | [...whisk/core/database/cosmosdb/CosmosDBSupport.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJTdXBwb3J0LnNjYWxh) | `0% <0%> (-74.08%)` | :arrow_down: | | [...a/org/apache/openwhisk/common/ExecutorCloser.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvbW1vbi9FeGVjdXRvckNsb3Nlci5zY2FsYQ==) | `0% <0%> (-66.67%)` | :arrow_down: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `4% <0%> (-52%)` | :arrow_down: | | ... and [7 more](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by
[GitHub] [openwhisk] dgrove-oss opened a new pull request #4629: remove spurious execute permission from ansible group_var files
dgrove-oss opened a new pull request #4629: remove spurious execute permission from ansible group_var files URL: https://github.com/apache/openwhisk/pull/4629 release pre-flight check...clear out some spurious execute mode bits on files. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk-wskdeploy] dgrove-oss edited a comment on issue #1073: Build non-source test artifacts from source
dgrove-oss edited a comment on issue #1073: Build non-source test artifacts from source URL: https://github.com/apache/openwhisk-wskdeploy/issues/1073#issuecomment-532226322 Note, when this issue is fixed, the exclusion in the release script package_source_code.sh addded in https://github.com/apache/openwhisk-release/pull/301 should be removed. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk-wskdeploy] dgrove-oss commented on issue #1073: Build non-source test artifacts from source
dgrove-oss commented on issue #1073: Build non-source test artifacts from source URL: https://github.com/apache/openwhisk-wskdeploy/issues/1073#issuecomment-532226322 Note, when this issue is fixed, the exclusion in the release script package_source_code.sh add in https://github.com/apache/openwhisk-release/pull/301 should be removed. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] codecov-io commented on issue #4628: Embedded Kafka support in OpenWhisk Standalone mode
codecov-io commented on issue #4628: Embedded Kafka support in OpenWhisk Standalone mode URL: https://github.com/apache/openwhisk/pull/4628#issuecomment-53154 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr=h1) Report > Merging [#4628](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/2036548e62dbf959d91c2328e86318bd7cfa656f?src=pr=desc) will **decrease** coverage by `5.74%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4628 +/- ## == - Coverage 84.44% 78.69% -5.75% == Files 183 183 Lines8305 8305 Branches 574 574 == - Hits 7013 6536 -477 - Misses 1292 1769 +477 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr=tree) | Coverage Δ | | |---|---|---| | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.89%)` | :arrow_down: | | [...tabase/cosmosdb/cache/CacheInvalidatorConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3JDb25maWcuc2NhbGE=) | `0% <0%> (-94.74%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | [...e/database/cosmosdb/cache/ChangeFeedListener.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NoYW5nZUZlZWRMaXN0ZW5lci5zY2FsYQ==) | `0% <0%> (-86.67%)` | :arrow_down: | | [...e/database/cosmosdb/cache/KafkaEventProducer.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0thZmthRXZlbnRQcm9kdWNlci5zY2FsYQ==) | `0% <0%> (-76.48%)` | :arrow_down: | | [...whisk/core/database/cosmosdb/CosmosDBSupport.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJTdXBwb3J0LnNjYWxh) | `0% <0%> (-74.08%)` | :arrow_down: | | [...a/org/apache/openwhisk/common/ExecutorCloser.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvbW1vbi9FeGVjdXRvckNsb3Nlci5zY2FsYQ==) | `0% <0%> (-66.67%)` | :arrow_down: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `4% <0%> (-52%)` | :arrow_down: | | ... and [7 more](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by
[GitHub] [openwhisk-release] dgrove-oss opened a new pull request #301: cli group 1.0.0 release config
dgrove-oss opened a new pull request #301: cli group 1.0.0 release config URL: https://github.com/apache/openwhisk-release/pull/301 1. Hack package_source_code.sh to workaround wskdeploy https://github.com/apache/openwhisk-wskdeploy/issues/1073 2. Add config.json for cli 1.0.0-rc1 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] rabbah commented on a change in pull request #4624: Combines active ack and slot release when both are available.
rabbah commented on a change in pull request #4624: Combines active ack and
slot release when both are available.
URL: https://github.com/apache/openwhisk/pull/4624#discussion_r325159952
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -64,43 +64,34 @@ case class ActivationMessage(override val transid:
TransactionId,
def causedBySequence: Boolean = cause.isDefined
}
-object ActivationMessage extends DefaultJsonProtocol {
-
- def parse(msg: String) = Try(serdes.read(msg.parseJson))
-
- private implicit val fqnSerdes = FullyQualifiedEntityName.serdes
- implicit val serdes = jsonFormat11(ActivationMessage.apply)
-}
-
/**
* Message that is sent from the invoker to the controller after action is
completed or after slot is free again for
* new actions.
*/
abstract class AcknowledegmentMessage(private val tid: TransactionId) extends
Message {
override val transid: TransactionId = tid
- override def serialize: String = {
-AcknowledegmentMessage.serdes.write(this).compactPrint
- }
+ override def serialize: String =
AcknowledegmentMessage.serdes.write(this).compactPrint
+ def toJson: JsValue
}
/**
- * This message is sent from the invoker to the controller, after the slot of
an invoker that has been used by the
- * current action, is free again (after log collection)
+ * This message is sent from an invoker to the controller, after the resource
slot in the invoke, used by the
+ * corresponding activation, is free again (i.e., after log collection). In
some cases, the activation result is
+ * ready and the slot is freed at the same time. In such cases, the completion
message carries the result as well.
+ * This is reflected by the of a Right() `response` and the param `result` is
set to true.
+ * In some cases the `result` is true but the response is Left() if the
message was too large for the message bus.
*/
case class CompletionMessage(override val transid: TransactionId,
- activationId: ActivationId,
- isSystemError: Boolean,
+ response: Either[ActivationId, WhiskActivation],
+ result: Boolean, // true iff the message is a
combined active ack and slot released
Review comment:
> part of a "system upgrade" documentation?
this project doesn't have any yet - are you proposing we start one? ;)
If we introduce a third type, then new invokers could send messages for the
bug fix, but an old controller would still ignore them (in fact it would be
worse since then the slot release is not recorded)... We could send 2 messages
to replicate the split phase notification, but this is wasteful IMO, but then
avoid the compatibility issue for a rolling update.
Do you want this PR to address this?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-wskdeploy] dgrove-oss opened a new issue #1073: Build non-source test artifacts from source
dgrove-oss opened a new issue #1073: Build non-source test artifacts from source URL: https://github.com/apache/openwhisk-wskdeploy/issues/1073 The directory tests/src/integration contains a number of non-source artifacts (.zip, .jar, compiled executables) that cannot be included in an Apache source release. A build target should be added to re-create all these files from source (and the artifacts should either be removed from git entirely or centralized to a single directory like tests/dat/ or tests/binary that can be cleanly excluded from source releases). The current list is: ``` scanning for unexpected file types... failed /openwhisk-wskdeploy-1.0.0/tests/src/integration/helloworld/actions/hello.zip: application/zip; charset=binary /openwhisk-wskdeploy-1.0.0/tests/src/integration/helloworld/actions/hello.jar: application/java-archive; charset=binary /openwhisk-wskdeploy-1.0.0/tests/src/integration/docker/actions/go/exec: application/x-executable; charset=binary /openwhisk-wskdeploy-1.0.0/tests/src/integration/docker/actions/exec.zip: application/zip; charset=binary /openwhisk-wskdeploy-1.0.0/tests/src/integration/validate-packages-in-manifest/actions/hello.jar: application/java-archive; charset=binary /openwhisk-wskdeploy-1.0.0/tests/src/integration/jaraction/src/hello.jar: application/java-archive; charset=binary /openwhisk-wskdeploy-1.0.0/tests/src/integration/runtimetests/src/helloworld/helloworld.zip: application/zip; charset=binary /openwhisk-wskdeploy-1.0.0/tests/src/integration/runtimetests/src/helloDotNet.src.zip: application/zip; charset=binary /openwhisk-wskdeploy-1.0.0/tests/src/integration/runtimetests/src/hello.jar: application/java-archive; charset=binary /openwhisk-wskdeploy-1.0.0/tests/src/integration/runtimetests/src/helloDotNet.zip: application/zip; charset=binary) scanning for archives ... failed (/openwhisk-wskdeploy-1.0.0/tests/src/integration/helloworld/actions/hello.jar /openwhisk-wskdeploy-1.0.0/tests/src/integration/validate-packages-in-manifest/actions/hello.jar /openwhisk-wskdeploy-1.0.0/tests/src/integration/jaraction/src/hello.jar /openwhisk-wskdeploy-1.0.0/tests/src/integration/runtimetests/src/hello.jar) ``` 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on issue #4627: Add descriptions how to update, remove or rename runtime kinds and language families
sven-lange-last commented on issue #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#issuecomment-532206912 @dgrove-oss thanks a lot for reviewing and providing clarifications / improvements. I incorporated your proposals. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
sven-lange-last commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325145651 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. Review comment: Thanks a lot for providing more accurate wording / terms. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
sven-lange-last commented on a change in pull request #4627: Add descriptions
how to update, remove or rename runtime kinds and language families
URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325145142
##
File path: ansible/files/runtimes-nodeonly.json
##
@@ -1,4 +1,18 @@
{
+"description": [
+"This file describes the different languages (aka. managed action
runtimes) supported by the system",
+"as well as blackbox images that support the runtime specification.",
+"Only actions with runtime families / kinds defined here can be
created / read / updated / deleted / invoked.",
+"Define a list of runtime families (example: 'nodejs') with at least
one kind per family (example: 'nodejs:10').",
+"Each runtime family needs a default kind (default: true).",
+"When removing or renaming runtime families or runtime kinds from this
file, all already existing actions",
Review comment:
Thanks a lot for providing more accurate wording / terms.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on issue #4626: Allow limiting DB bloat by excluding response from Activation record in some cases
sven-lange-last commented on issue #4626: Allow limiting DB bloat by excluding response from Activation record in some cases URL: https://github.com/apache/openwhisk/issues/4626#issuecomment-532203874 Thanks for the proposal - it makes a lot of sense. We also see house-keeping problems with activations in CouchDB - we actually use Cloudant. A concept that fully removes activation documents from CouchDB after a retention period is complex and consumes a lot of CPU on the database cluster. In addition, a large amount of activation documents consumes a lot of space. As a consequence, storing activations can get pretty expensive if you run a large-scale production environment. When going forward with the proposal of not storing activations for blocking + successful + "not timed out on the front-end" invocations, we need to check with clients whether their existing workloads break with such a change. Migration and evolution of the system is always the challenge of running a public production environment. We had in mind to introduce a kind of activation store throttling - for example, only store a limited amount of activations per minute. This gives you a nice developer experience when starting with Openwhisk and for trying things out. But when running large production workloads, the workload owner would have to take care of storing action results - it's no more done by the platform. Your proposal with not storing blocking + successful + "not timed out" invocations has the big advantage that you will either receive activation results as result of your blocking activation or by explicitly asking for activation results. With activation store throttling, there are situations where the platform does not give you the activation result at all. Regarding the `ElasticSearchActivationStore` - we recently moved from an ELK based logging stack to LogDNA which bases on a different technology. That's why the `ElasticSearchActivationStore` is no more in focus for us. At the moment, we are storing activations to our databases as well as forwarding them to the client's LogDNA logging space. I think there are two valid strategies in this area: reducing the amount of activations as well as using better options for storing activations. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] chetanmeh opened a new pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode
chetanmeh opened a new pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode URL: https://github.com/apache/openwhisk/pull/4628 Adds support for launching Embedded Kafka and Kafka Drop UI ## Description This PR adds support for launching an [embedded-kafka][1]. This is mostly meant to enable local development for features which need Kafka support (like User Event or Activation Persister Service etc). In this mode an inprocess Kafka Server (v2.1.1 currently) and Zookeeper Server ``` java -jar openwhisk-standalone.jar --kafka ``` Key aspects * Kafka version 2.1.1 is being used while we run with 0.11.0.1. Tried to use that but then it causes class compatability issue with newer version of Kafka client being used * This mode is also useful to simulate some of the integration test scenarios with Activation Persister Service * Kafka Server launched is supporting connection from both host and within Docker container based on [kafka listeners explanation][3] * Kafka and Zookeeper generated files are stored in `~/.openwhisk/standalone/server-3233/tmp` and are removed upon restart Once launched you can see the details of port used etc as below ``` [ 9092 ] localhost:9092 (kafka) [ 9092 ] 192.168.65.2:9091 (kafka-docker) [ 2181 ] Zookeeper (zookeeper) [ 9000 ] http://localhost:9000 (whisk-kafka-drop-ui) ``` ### Kafka Ui - Kafdrop 3 This PR also adds support for launching an optional [Kafdrop 3][2] ``` java -jar openwhisk-standalone.jar --kafka --kafka-ui ``` This UI can help a developer to understand the structure of messages exchanged on various topics easily  ## Related issue and scope - [ ] I opened an issue to propose and discuss this change (#) ## My changes affect the following components - [ ] API - [ ] Controller - [ ] Message Bus (e.g., Kafka) - [ ] Loadbalancer - [ ] Invoker - [ ] Intrinsic actions (e.g., sequences, conductors) - [ ] Data stores (e.g., CouchDB) - [ ] Tests - [ ] Deployment - [ ] CLI - [ ] General tooling - [ ] Documentation ## Types of changes - [ ] Bug fix (generally a non-breaking change which closes an issue). - [ ] Enhancement or new feature (adds new functionality). - [ ] Breaking change (a bug fix or enhancement which changes existing behavior). ## Checklist: - [ ] I signed an [Apache CLA](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md). - [ ] I reviewed the [style guides](https://github.com/apache/openwhisk/wiki/Contributing:-Git-guidelines#code-readiness) and followed the recommendations (Travis CI will check :). - [ ] I added tests to cover my changes. - [ ] My changes require further changes to the documentation. - [ ] I updated the documentation where necessary. [1]: https://github.com/embeddedkafka/embedded-kafka [2]: https://github.com/obsidiandynamics/kafdrop [3]: https://rmoff.net/2018/08/02/kafka-listeners-explained/ 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325136281 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. + +### Updating runtimes + +Follow these steps to update a particular runtime kind: + +1. Update the runtimes' container image. +2. Update the corresponding `image` property in the [runtimes manifest](actions-new.md#the-runtimes-manifest) to use the new container image. +3. Restart / re-deploy controllers and invokers such that they pick up the changed runtimes manifest. + +Already existing actions of the changed runtime kind will immediately use the new container image when the invoker creates a new action container. + +Apparently, this approach should only be used if existing actions do not break with the new container image. If a new container image is supposed to break existing actions, introduce a new runtime kind. + +### Removing runtimes + +Follow these steps to remove a particular runtime kind under the assumption that actions with the runtime kind exist in the system. Apparently, it makes sense to wait some time between accomplishing these steps to give action owners time to react. + +1. Deprecate the runtime kind by setting `"deprecated": true` in the [runtimes manifest](actions-new.md#the-runtimes-manifest). This setting prevents that new actions can be created with the deprecated action kind. In addition, existing actions cannot be changed to the deprecated action kind any more. +2. Ask owners of existing actions with runtime kind to be removed to update their actions to a different action kind. +3. Create an automated process that identifies all actions with the runtime kind to be removed in the system's action artifact store. Either automatically remove these actions or change to a different runtime kind. +4. Once the system's action artifact store does no more contain actions with the runtime kind to be removed, remove the runtime kind from the [runtimes manifest](actions-new.md#the-runtimes-manifest). +5. TODO - is this correct?: Remove the runtime kind from the list of known kinds in the `ActionExec` object of the [controller API's Swagger definition](../core/controller/src/main/resources/apiv1swagger.json). + +If you remove a runtime kind from the [runtimes manifest](actions-new.md#the-runtimes-manifest), all actions that still use the removed runtime kind can no more be read from the system's action artifact store and thus, can no more be read, updated, deleted or invoked. Review comment: "can no more" ==> "can no longer" 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325135023 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. + +### Updating runtimes + +Follow these steps to update a particular runtime kind: + +1. Update the runtimes' container image. +2. Update the corresponding `image` property in the [runtimes manifest](actions-new.md#the-runtimes-manifest) to use the new container image. +3. Restart / re-deploy controllers and invokers such that they pick up the changed runtimes manifest. + +Already existing actions of the changed runtime kind will immediately use the new container image when the invoker creates a new action container. + +Apparently, this approach should only be used if existing actions do not break with the new container image. If a new container image is supposed to break existing actions, introduce a new runtime kind. Review comment: Apparently ==> Obviously ? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325134733 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. Review comment: ...to prevent problems with preexisting actions. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325135978 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. + +### Updating runtimes + +Follow these steps to update a particular runtime kind: + +1. Update the runtimes' container image. +2. Update the corresponding `image` property in the [runtimes manifest](actions-new.md#the-runtimes-manifest) to use the new container image. +3. Restart / re-deploy controllers and invokers such that they pick up the changed runtimes manifest. + +Already existing actions of the changed runtime kind will immediately use the new container image when the invoker creates a new action container. + +Apparently, this approach should only be used if existing actions do not break with the new container image. If a new container image is supposed to break existing actions, introduce a new runtime kind. + +### Removing runtimes + +Follow these steps to remove a particular runtime kind under the assumption that actions with the runtime kind exist in the system. Apparently, it makes sense to wait some time between accomplishing these steps to give action owners time to react. + +1. Deprecate the runtime kind by setting `"deprecated": true` in the [runtimes manifest](actions-new.md#the-runtimes-manifest). This setting prevents that new actions can be created with the deprecated action kind. In addition, existing actions cannot be changed to the deprecated action kind any more. +2. Ask owners of existing actions with runtime kind to be removed to update their actions to a different action kind. +3. Create an automated process that identifies all actions with the runtime kind to be removed in the system's action artifact store. Either automatically remove these actions or change to a different runtime kind. +4. Once the system's action artifact store does no more contain actions with the runtime kind to be removed, remove the runtime kind from the [runtimes manifest](actions-new.md#the-runtimes-manifest). Review comment: "does no more" ==> "does not" 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
dgrove-oss commented on a change in pull request #4627: Add descriptions how to
update, remove or rename runtime kinds and language families
URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325134141
##
File path: ansible/files/runtimes-nodeonly.json
##
@@ -1,4 +1,18 @@
{
+"description": [
+"This file describes the different languages (aka. managed action
runtimes) supported by the system",
+"as well as blackbox images that support the runtime specification.",
+"Only actions with runtime families / kinds defined here can be
created / read / updated / deleted / invoked.",
+"Define a list of runtime families (example: 'nodejs') with at least
one kind per family (example: 'nodejs:10').",
+"Each runtime family needs a default kind (default: true).",
+"When removing or renaming runtime families or runtime kinds from this
file, all already existing actions",
Review comment:
Wordsmithing:
"When removing or renaming runtime families or runtime kinds from
this file, all preexisting actions",
"with the affected kinds can no longer be read / updated / deleted /
invoked. In order to remove or rename
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325135738 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. + +### Updating runtimes + +Follow these steps to update a particular runtime kind: + +1. Update the runtimes' container image. +2. Update the corresponding `image` property in the [runtimes manifest](actions-new.md#the-runtimes-manifest) to use the new container image. +3. Restart / re-deploy controllers and invokers such that they pick up the changed runtimes manifest. + +Already existing actions of the changed runtime kind will immediately use the new container image when the invoker creates a new action container. + +Apparently, this approach should only be used if existing actions do not break with the new container image. If a new container image is supposed to break existing actions, introduce a new runtime kind. + +### Removing runtimes + +Follow these steps to remove a particular runtime kind under the assumption that actions with the runtime kind exist in the system. Apparently, it makes sense to wait some time between accomplishing these steps to give action owners time to react. Review comment: Apparently isn't the right word here. Perhaps change to "Clearly the steps below should be spaced out in time to give action owners..." 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] dgrove-oss commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
dgrove-oss commented on a change in pull request #4627: Add descriptions how to
update, remove or rename runtime kinds and language families
URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325136658
##
File path: ansible/files/runtimes.json
##
@@ -1,4 +1,17 @@
{
+"description": [
+"This file describes the different languages (aka. managed action
runtimes) supported by the system",
+"as well as blackbox images that support the runtime specification.",
+"Only actions with runtime families / kinds defined here can be
created / read / updated / deleted / invoked.",
+"Define a list of runtime families (example: 'nodejs') with at least
one kind per family (example: 'nodejs:10').",
+"Each runtime family needs a default kind (default: true).",
+"When removing or renaming runtime families or runtime kinds from this
file, all already existing actions",
+"with the affected kinds can no more be read / updated / deleted /
invoked. In order to remove or rename",
Review comment:
Same suggestions as other copy of this text
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] rabbah commented on issue #4626: Allow limiting DB bloat by excluding response from Activation record in some cases
rabbah commented on issue #4626: Allow limiting DB bloat by excluding response from Activation record in some cases URL: https://github.com/apache/openwhisk/issues/4626#issuecomment-532160191 +1 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] codecov-io commented on issue #4627: Add descriptions how to update, remove or rename runtime kinds and language families
codecov-io commented on issue #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#issuecomment-532154575 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr=h1) Report > Merging [#4627](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/2036548e62dbf959d91c2328e86318bd7cfa656f?src=pr=desc) will **decrease** coverage by `5.67%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4627 +/- ## == - Coverage 84.44% 78.77% -5.68% == Files 183 183 Lines8305 8305 Branches 574 574 == - Hits 7013 6542 -471 - Misses 1292 1763 +471 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr=tree) | Coverage Δ | | |---|---|---| | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.89%)` | :arrow_down: | | [...tabase/cosmosdb/cache/CacheInvalidatorConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3JDb25maWcuc2NhbGE=) | `0% <0%> (-94.74%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | [...e/database/cosmosdb/cache/ChangeFeedListener.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NoYW5nZUZlZWRMaXN0ZW5lci5zY2FsYQ==) | `0% <0%> (-86.67%)` | :arrow_down: | | [...e/database/cosmosdb/cache/KafkaEventProducer.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0thZmthRXZlbnRQcm9kdWNlci5zY2FsYQ==) | `0% <0%> (-76.48%)` | :arrow_down: | | [...whisk/core/database/cosmosdb/CosmosDBSupport.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJTdXBwb3J0LnNjYWxh) | `0% <0%> (-74.08%)` | :arrow_down: | | [...a/org/apache/openwhisk/common/ExecutorCloser.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvbW1vbi9FeGVjdXRvckNsb3Nlci5zY2FsYQ==) | `0% <0%> (-66.67%)` | :arrow_down: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `4% <0%> (-52%)` | :arrow_down: | | ... and [7 more](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by
[GitHub] [openwhisk] BillZong commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
BillZong commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325014559 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. + +### Updating runtimes + +Follow these steps to update a particular runtime kind: + +1. Update the runtimes' container image. +2. Update the corresponding `image` property in the [runtimes manifest](actions-new.md#the-runtimes-manifest) to use the new container image. +3. Restart / re-deploy controllers and invokers such that they pick up the changed runtimes manifest. + +Already existing actions of the changed runtime kind will immediately use the new container image when the invoker creates a new action container. + +Apparently, this approach should only be used if existing actions do not break with the new container image. If a new container image is supposed to break existing actions, introduce a new runtime kind. + +### Removing runtimes + +Follow these steps to remove a particular runtime kind under the assumption that actions with the runtime kind exist in the system. Apparently, it makes sense to wait some time between accomplishing these steps to give action owners time to react. + +1. Deprecate the runtime kind by setting `"deprecated": true` in the [runtimes manifest](actions-new.md#the-runtimes-manifest). This setting prevents that new actions can be created with the deprecated action kind. In addition, existing actions cannot be changed to the deprecated action kind any more. +2. Ask owners of existing actions with runtime kind to be removed to update their actions to a different action kind. +3. Create an automated process that identifies all actions with the runtime kind to be removed in the system's action artifact store. Either automatically remove these actions or change to a different runtime kind. +4. Once the system's action artifact store does no more contain actions with the runtime kind to be removed, remove the runtime kind from the [runtimes manifest](actions-new.md#the-runtimes-manifest). +5. TODO - is this correct?: Remove the runtime kind from the list of known kinds in the `ActionExec` object of the [controller API's Swagger definition](../core/controller/src/main/resources/apiv1swagger.json). Review comment: You're right about this, the `Controller.info()` use runtimes manifest. So the `ActionExec` in `apiv1swagger.json` would only make sense when executing unit tests. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] style95 commented on issue #4626: Allow limiting DB bloat by excluding response from Activation record in some cases
style95 commented on issue #4626: Allow limiting DB bloat by excluding response from Activation record in some cases URL: https://github.com/apache/openwhisk/issues/4626#issuecomment-532092188 Regarding this issue, I think the main reason is the limited scalability of CouchDB. If CouchDB receives too many requests, it cannot handle them properly and even crashes sometimes. Also, more and more data is stored, more and more CouchDB would be highly likely unavailable. It lacks the functionality to manage existing old(unused) data. As OW depends on "view" of CouchDB, we need to periodically trigger indexing of views to prevent CouchDB from the crash while it indexes too much data. If the backend store is scaling enough to handle all data and easy to delete unnecessary(old) data, it would solve many parts of this issue. For example, if we introduce a scalable datastore such as ElasticSearch, it would greatly alleviate the situation. I have observed many issues with CouchDB and we here(Naver) replaced it with ElasticSearch and observed no issue so far. It outperforms CouchDB in many aspects such as : 1. Enable full-text search of activation(including logs) 2. Better scalability 3. Easy to manipulate data especially in terms of deleting old(unused) data with the index alias. AFAIK, @dubee had worked on `ElasticSearchActivaitonStore` some time back, but it was not completed for some reasons. I suspect there may be some historical reason to tie up with CouchDB? If there is no other historical reason to persist CouchDB, I would like to bring up the `ElasticSearchActivationStore` again and we are willing to contribute it as well. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
sven-lange-last commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families URL: https://github.com/apache/openwhisk/pull/4627#discussion_r325004619 ## File path: docs/actions-update.md ## @@ -0,0 +1,75 @@ + +## Updating Action Language Runtimes + +OpenWhisk supports [several languages and runtimes](actions.md#languages-and-runtimes) that can be made +available for usage in an OpenWhisk deployment. This is done via the [runtimes manifest](actions-new.md#the-runtimes-manifest). + +Over time, you may have the need for change: + +* Update runtimes to address security issues - for example, the latest code level of Node.js 10. +* Remove runtime versions that are no more supported - for example, Node.js 6. +* Add more languages due to user demand. +* Remove languages that are no more needed. + +While adding and updating languages and runtimes is pretty straightforward, removing or renaming languages and runtimes +requires some caution to prevent problems with existing actions. + +### Updating runtimes + +Follow these steps to update a particular runtime kind: + +1. Update the runtimes' container image. +2. Update the corresponding `image` property in the [runtimes manifest](actions-new.md#the-runtimes-manifest) to use the new container image. +3. Restart / re-deploy controllers and invokers such that they pick up the changed runtimes manifest. + +Already existing actions of the changed runtime kind will immediately use the new container image when the invoker creates a new action container. + +Apparently, this approach should only be used if existing actions do not break with the new container image. If a new container image is supposed to break existing actions, introduce a new runtime kind. + +### Removing runtimes + +Follow these steps to remove a particular runtime kind under the assumption that actions with the runtime kind exist in the system. Apparently, it makes sense to wait some time between accomplishing these steps to give action owners time to react. + +1. Deprecate the runtime kind by setting `"deprecated": true` in the [runtimes manifest](actions-new.md#the-runtimes-manifest). This setting prevents that new actions can be created with the deprecated action kind. In addition, existing actions cannot be changed to the deprecated action kind any more. +2. Ask owners of existing actions with runtime kind to be removed to update their actions to a different action kind. +3. Create an automated process that identifies all actions with the runtime kind to be removed in the system's action artifact store. Either automatically remove these actions or change to a different runtime kind. +4. Once the system's action artifact store does no more contain actions with the runtime kind to be removed, remove the runtime kind from the [runtimes manifest](actions-new.md#the-runtimes-manifest). +5. TODO - is this correct?: Remove the runtime kind from the list of known kinds in the `ActionExec` object of the [controller API's Swagger definition](../core/controller/src/main/resources/apiv1swagger.json). Review comment: I'm not sure where the Swagger is exactly used. The controller itself offers an endpoint where it publishes the supported runtimes - and this seems to base on the runtimes manifest, not the Swagger definition. See for example: https://us-south.functions.cloud.ibm.com/ 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: us...@infra.apache.org With regards, Apache Git Services
