[GitHub] [openwhisk] sven-lange-last commented on issue #4633: Optimize WhiskEntity deserialization
sven-lange-last commented on issue #4633: Optimize WhiskEntity deserialization URL: https://github.com/apache/openwhisk/issues/4633#issuecomment-532997277 This approach would improve scenarios where pre-existing actions are loaded from the artifact store and fail in deserialisation because the used runtime kind has been removed. In this scenario, the document cannot be deserialised into a WhiskAction and will be read as WhiskTrigger. See https://lists.apache.org/thread.html/16cc3717025203c071dfd3ceb6652c98ec8e56737c4ac57dbb2e6021@%3Cdev.openwhisk.apache.org%3E 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_r326038270
##
File path:
core/controller/src/main/scala/org/apache/openwhisk/core/loadBalancer/CommonLoadBalancer.scala
##
@@ -204,17 +204,20 @@ abstract class CommonLoadBalancer(config: WhiskConfig,
protected[loadBalancer] def processAcknowledgement(bytes: Array[Byte]):
Future[Unit] = Future {
val raw = new String(bytes, StandardCharsets.UTF_8)
AcknowledegmentMessage.parse(raw) match {
- case Success(m: CompletionMessage) =>
-processCompletion(
- m.activationId,
- m.transid,
- forced = false,
- isSystemError = m.isSystemError,
- invoker = m.invoker)
-activationFeed ! MessageFeed.Processed
+ case Success(acknowledegment) =>
+acknowledegment.isSlotFree.foreach { invoker =>
+ processCompletion(
+acknowledegment.activationId,
+acknowledegment.transid,
+forced = false,
+isSystemError = acknowledegment.isSystemError.getOrElse(false),
+invoker)
+}
+
+acknowledegment.result.foreach { response =>
+ processResult(acknowledegment.activationId, acknowledegment.transid,
response)
+}
Review comment:
Pretty elegant way to fold all three types of message into a single flow
because `acknowledegment.isSlotFree` and `acknowledegment.isSlotFree` are
options.
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_r326038270
##
File path:
core/controller/src/main/scala/org/apache/openwhisk/core/loadBalancer/CommonLoadBalancer.scala
##
@@ -204,17 +204,20 @@ abstract class CommonLoadBalancer(config: WhiskConfig,
protected[loadBalancer] def processAcknowledgement(bytes: Array[Byte]):
Future[Unit] = Future {
val raw = new String(bytes, StandardCharsets.UTF_8)
AcknowledegmentMessage.parse(raw) match {
- case Success(m: CompletionMessage) =>
-processCompletion(
- m.activationId,
- m.transid,
- forced = false,
- isSystemError = m.isSystemError,
- invoker = m.invoker)
-activationFeed ! MessageFeed.Processed
+ case Success(acknowledegment) =>
+acknowledegment.isSlotFree.foreach { invoker =>
+ processCompletion(
+acknowledegment.activationId,
+acknowledegment.transid,
+forced = false,
+isSystemError = acknowledegment.isSystemError.getOrElse(false),
+invoker)
+}
+
+acknowledegment.result.foreach { response =>
+ processResult(acknowledegment.activationId, acknowledegment.transid,
response)
+}
Review comment:
Pretty elegant way to fold all three types of message into a single flow
because `acknowledegment.isSlotFree` and `acknowledegment.result` are options.
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_r326048416
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -64,100 +64,165 @@ 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
+
+ /** Pithy descriptor for logging. */
+ def name: String
+
+ /** Does message indicate slot is free? */
+ def isSlotFree: Option[InvokerInstanceId]
+
+ /** Does message contain a result? */
+ def result: Option[Either[ActivationId, WhiskActivation]]
+
+ /**
+ * Is the acknowledgement for an activation that failed internally?
+ * For some message, this is not relevant and the result is None.
+ */
+ def isSystemError: Option[Boolean]
+
+ def activationId: ActivationId
+
+ /** Serializes the message to JSON. */
+ def toJson: JsValue
+
+ /**
+ * Converts the message to a more compact form if it cannot cross the
message bus as is or some of its details are not necessary.
+ */
+ def shrink: AcknowledegmentMessage
}
/**
- * 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 in situtations when
the resource slot and the action
+ * result are available at the same time, and so the split-phase notification
is not necessary. Instead the message
+ * combines the `CompletionMessage` and `ResultMessage`. The `response` may be
an `ActivationId` to allow for failures
+ * to send the activation result because of event-bus size limitations.
*/
-case class CompletionMessage(override val transid: TransactionId,
- activationId: ActivationId,
- isSystemError: Boolean,
- invoker: InvokerInstanceId)
+case class CombinedCompletionAndResultMessage(override val transid:
TransactionId,
+ response: Either[ActivationId,
WhiskActivation],
+ override val isSystemError:
Option[Boolean],
+ invoker: InvokerInstanceId)
extends AcknowledegmentMessage(transid) {
-
- override def toString = {
-activationId.asString
- }
+ override def name = "combined"
+ override def result = Some(response)
+ override def isSlotFree = Some(invoker)
+ override def activationId = response.fold(identity, _.activationId)
+ override def toJson = CombinedCompletionAndResultMessage.serdes.write(this)
+ override def shrink = copy(response = response.flatMap(a =>
Left(a.activationId)))
+ override def toString = response.fold(identity, _.activationId).asString
}
-object CompletionMessage extends DefaultJsonProtocol {
- def parse(msg: String): Try[CompletionMessage] =
Try(serdes.read(msg.parseJson))
- implicit val serdes = jsonFormat4(CompletionMessage.apply)
+/**
+ * This message is sent from an invoker to the controller, once the resource
slot in the invoker (used by the
+ * corresponding activation) free again (i.e., after log collection). The
`CompletionMessage` is part of a split
+ * phase notification to the load balancer where an invoker first sends a
`ResultMessage` and later sends the
+ * `CompletionMessage`.
+ */
+case class CompletionMessage(override val transid: TransactionId,
+ override val activationId: ActivationId,
+ override val isSystemError: Option[Boolean],
+ invoker: InvokerInstanceId)
+extends AcknowledegmentMessage(transid) {
+ override def name = "completion"
+ override def result = None
+ override def isSlotFree = Some(invoker)
+ override def toJson = CompletionMessage.serdes.write(this)
+ override def shrink = this
+ override def toString = activationId.asString
}
/**
- * That message will be sent from the invoker to the controller after action
completion if the user wants to have
- * the result immediately (blocking activa
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh 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_r326054019
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -64,100 +64,165 @@ 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
+
+ /** Pithy descriptor for logging. */
+ def name: String
+
+ /** Does message indicate slot is free? */
+ def isSlotFree: Option[InvokerInstanceId]
+
+ /** Does message contain a result? */
+ def result: Option[Either[ActivationId, WhiskActivation]]
+
+ /**
+ * Is the acknowledgement for an activation that failed internally?
+ * For some message, this is not relevant and the result is None.
+ */
+ def isSystemError: Option[Boolean]
+
+ def activationId: ActivationId
+
+ /** Serializes the message to JSON. */
+ def toJson: JsValue
+
+ /**
+ * Converts the message to a more compact form if it cannot cross the
message bus as is or some of its details are not necessary.
+ */
+ def shrink: AcknowledegmentMessage
}
/**
- * 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 in situtations when
the resource slot and the action
+ * result are available at the same time, and so the split-phase notification
is not necessary. Instead the message
+ * combines the `CompletionMessage` and `ResultMessage`. The `response` may be
an `ActivationId` to allow for failures
+ * to send the activation result because of event-bus size limitations.
*/
-case class CompletionMessage(override val transid: TransactionId,
- activationId: ActivationId,
- isSystemError: Boolean,
- invoker: InvokerInstanceId)
+case class CombinedCompletionAndResultMessage(override val transid:
TransactionId,
+ response: Either[ActivationId,
WhiskActivation],
+ override val isSystemError:
Option[Boolean],
+ invoker: InvokerInstanceId)
extends AcknowledegmentMessage(transid) {
-
- override def toString = {
-activationId.asString
- }
+ override def name = "combined"
+ override def result = Some(response)
+ override def isSlotFree = Some(invoker)
+ override def activationId = response.fold(identity, _.activationId)
+ override def toJson = CombinedCompletionAndResultMessage.serdes.write(this)
+ override def shrink = copy(response = response.flatMap(a =>
Left(a.activationId)))
+ override def toString = response.fold(identity, _.activationId).asString
}
-object CompletionMessage extends DefaultJsonProtocol {
- def parse(msg: String): Try[CompletionMessage] =
Try(serdes.read(msg.parseJson))
- implicit val serdes = jsonFormat4(CompletionMessage.apply)
+/**
+ * This message is sent from an invoker to the controller, once the resource
slot in the invoker (used by the
+ * corresponding activation) free again (i.e., after log collection). The
`CompletionMessage` is part of a split
+ * phase notification to the load balancer where an invoker first sends a
`ResultMessage` and later sends the
+ * `CompletionMessage`.
+ */
+case class CompletionMessage(override val transid: TransactionId,
+ override val activationId: ActivationId,
+ override val isSystemError: Option[Boolean],
+ invoker: InvokerInstanceId)
+extends AcknowledegmentMessage(transid) {
+ override def name = "completion"
+ override def result = None
+ override def isSlotFree = Some(invoker)
+ override def toJson = CompletionMessage.serdes.write(this)
+ override def shrink = this
+ override def toString = activationId.asString
}
/**
- * That message will be sent from the invoker to the controller after action
completion if the user wants to have
- * the result immediately (blocking activation).
[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_r326063425
##
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala
##
@@ -145,43 +146,34 @@ class InvokerReactive(
new MessageFeed("activation", logging, consumer, maxPeek, 1.second,
processActivationMessage)
})
- /** Sends an active-ack. */
private val ack: InvokerReactive.ActiveAck = (tid: TransactionId,
activationResult:
WhiskActivation,
blockingInvoke: Boolean,
controllerInstance:
ControllerInstanceId,
userId: UUID,
-isSlotFree: Boolean) => {
+acknowledegment:
AcknowledegmentMessage) => {
Review comment:
If the passed `acknowledegment` is a combined completion and result message
or result message, it likely contains a `WhiskActivation` instance. So it
duplicates the `activationResult` parameter which is only used to emit user
metrics.
Can't we drop the `activationResult` parameter and change the condition for
sending user metrics? If the `acknowledegment.result` is non-empty and a
`Right(WhiskActivation)`, we emit metrics - otherwise 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] chetanmeh commented on a change in pull request #4584: OpenWhisk User Events
chetanmeh commented on a change in pull request #4584: OpenWhisk User Events
URL: https://github.com/apache/openwhisk/pull/4584#discussion_r326071229
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/entity/ActivationResult.scala
##
@@ -61,16 +61,25 @@ protected[core] object ActivationResponse extends
DefaultJsonProtocol {
val DeveloperError = 2 // action ran but failed to handle an error, or
action did not run and failed to initialize
val WhiskError = 3 // internal system error
- protected[core] def messageForCode(code: Int) = {
-require(code >= Success && code <= WhiskError)
+ val statusSuccess = "success"
+ val statusApplicationError = "application_error"
+ val statusDeveloperError = "action_developer_error"
+ val statusWhiskError = "whisk_internal_error"
+
+ protected[core] def statusForCode(code: Int) = {
+require(code >= 0 && code <= 3)
code match {
- case Success => "success"
- case ApplicationError => "application error"
- case DeveloperError => "action developer error"
- case WhiskError => "whisk internal error"
+ case 0 => ActivationResponse.statusSuccess
+ case 1 => ActivationResponse.statusApplicationError
+ case 2 => ActivationResponse.statusDeveloperError
+ case 3 => ActivationResponse.statusWhiskError
}
}
+ protected[core] def messageForCode(code: Int) = {
Review comment:
nit: `messageForCode` is called in critical frequent use path. So to avoid
unnecessary string computation it may be better to also keep the computed
values as val and just reuse them
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 commented on a change in pull request #4584: OpenWhisk User Events
chetanmeh commented on a change in pull request #4584: OpenWhisk User Events URL: https://github.com/apache/openwhisk/pull/4584#discussion_r326072530 ## File path: common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskActivation.scala ## @@ -145,6 +145,10 @@ object WhiskActivation val conductorAnnotation = "conductor" val timeoutAnnotation = "timeout" + val memoryAnnotation = "memory" Review comment: These are not annotations. So keep the name without `annotation` suffix 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 commented on a change in pull request #4584: OpenWhisk User Events
chetanmeh commented on a change in pull request #4584: OpenWhisk User Events
URL: https://github.com/apache/openwhisk/pull/4584#discussion_r326067606
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -256,6 +286,7 @@ case class Metric(metricName: String, metricValue: Long)
extends EventMessageBod
}
object Metric extends DefaultJsonProtocol {
+ val typeName = "Metric"
Review comment:
Probably not used. We have `typeName` in class also
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 commented on a change in pull request #4584: OpenWhisk User Events
chetanmeh commented on a change in pull request #4584: OpenWhisk User Events
URL: https://github.com/apache/openwhisk/pull/4584#discussion_r326065254
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -195,34 +198,59 @@ object EventMessageBody extends DefaultJsonProtocol {
case class Activation(name: String,
statusCode: Int,
- duration: Long,
- waitTime: Long,
- initTime: Long,
+ duration: Duration,
+ waitTime: Duration,
+ initTime: Duration,
kind: String,
conductor: Boolean,
memory: Int,
causedBy: Option[String])
extends EventMessageBody {
- val typeName = "Activation"
+ val typeName = Activation.typeName
override def serialize = toJson.compactPrint
+ def entityPath = EntityPath(name).toFullyQualifiedEntityName
Review comment:
Nit: Define return type for public methods. Like `entityPath`
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 commented on a change in pull request #4584: OpenWhisk User Events
chetanmeh commented on a change in pull request #4584: OpenWhisk User Events
URL: https://github.com/apache/openwhisk/pull/4584#discussion_r326069009
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/entity/ActivationResult.scala
##
@@ -61,16 +61,25 @@ protected[core] object ActivationResponse extends
DefaultJsonProtocol {
val DeveloperError = 2 // action ran but failed to handle an error, or
action did not run and failed to initialize
val WhiskError = 3 // internal system error
- protected[core] def messageForCode(code: Int) = {
-require(code >= Success && code <= WhiskError)
+ val statusSuccess = "success"
+ val statusApplicationError = "application_error"
+ val statusDeveloperError = "action_developer_error"
+ val statusWhiskError = "whisk_internal_error"
+
+ protected[core] def statusForCode(code: Int) = {
+require(code >= 0 && code <= 3)
code match {
- case Success => "success"
- case ApplicationError => "application error"
- case DeveloperError => "action developer error"
- case WhiskError => "whisk internal error"
+ case 0 => ActivationResponse.statusSuccess
Review comment:
Better to retain use of existing constants.
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 commented on a change in pull request #4584: OpenWhisk User Events
chetanmeh commented on a change in pull request #4584: OpenWhisk User Events
URL: https://github.com/apache/openwhisk/pull/4584#discussion_r326119356
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -195,34 +198,59 @@ object EventMessageBody extends DefaultJsonProtocol {
case class Activation(name: String,
statusCode: Int,
- duration: Long,
- waitTime: Long,
- initTime: Long,
+ duration: Duration,
+ waitTime: Duration,
+ initTime: Duration,
kind: String,
conductor: Boolean,
memory: Int,
causedBy: Option[String])
extends EventMessageBody {
- val typeName = "Activation"
+ val typeName = Activation.typeName
override def serialize = toJson.compactPrint
+ def entityPath = EntityPath(name).toFullyQualifiedEntityName
def toJson = Activation.activationFormat.write(this)
+
+ def status: String = statusForCode(statusCode)
+
+ def isColdStart: Boolean = initTime != Duration.Zero
+
+ def namespace: String = entityPath.path.root.name
+
+ def action: String = entityPath.fullPath.relativePath.get.namespace
+
}
object Activation extends DefaultJsonProtocol {
+
+ val typeName = "Activation"
def parse(msg: String) = Try(activationFormat.read(msg.parseJson))
+
+ private implicit val durationFormat = new RootJsonFormat[Duration] {
+override def write(obj: Duration): JsValue = obj match {
+ case o if o.isFinite => JsNumber(o.toMillis)
+ case _ => JsNumber.zero
+}
+
+override def read(json: JsValue): Duration = json match {
+ case JsNumber(n) if n <= 0 => Duration.Zero
+ case JsNumber(n) => toDuration(n.longValue)
+}
+ }
+
implicit val activationFormat =
jsonFormat(
Activation.apply _,
"name",
- "statusCode",
- "duration",
- "waitTime",
- "initTime",
- "kind",
- "conductor",
- "memory",
- "causedBy")
+ WhiskActivation.statusCodeAnnotation,
Review comment:
This can be avoided as it does not serve much purpose to use those
annotations value as constant (some are not annotation also like memory). So it
would be fine to revert to preexisting way
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 commented on a change in pull request #4584: OpenWhisk User Events
chetanmeh commented on a change in pull request #4584: OpenWhisk User Events
URL: https://github.com/apache/openwhisk/pull/4584#discussion_r326119692
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskActivation.scala
##
@@ -82,8 +82,8 @@ case class WhiskActivation(namespace: EntityPath,
if (end != Instant.EPOCH) {
Map(
"end" -> end.toJson,
- "duration" -> (duration getOrElse (end.toEpochMilli -
start.toEpochMilli)).toJson,
- "statusCode" -> response.statusCode.toJson)
+ WhiskActivation.durationAnnotation -> (duration getOrElse
(end.toEpochMilli - start.toEpochMilli)).toJson,
Review comment:
This change of using existing constants can be avoided. Using string
literals is fine here
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 #4628: Embedded Kafka support in OpenWhisk Standalone mode
sven-lange-last commented on a change in pull request #4628: Embedded Kafka
support in OpenWhisk Standalone mode
URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326149173
##
File path:
core/standalone/src/main/scala/org/apache/openwhisk/standalone/KafkaLauncher.scala
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.standalone
+
+import java.io.File
+
+import akka.actor.ActorSystem
+import akka.stream.ActorMaterializer
+import kafka.server.KafkaConfig
+import net.manub.embeddedkafka.{EmbeddedKafka, EmbeddedKafkaConfig}
+import org.apache.commons.io.FileUtils
+import org.apache.openwhisk.common.{Logging, TransactionId}
+import org.apache.openwhisk.core.WhiskConfig
+import org.apache.openwhisk.core.WhiskConfig.kafkaHosts
+import org.apache.openwhisk.core.entity.ControllerInstanceId
+import org.apache.openwhisk.core.loadBalancer.{LeanBalancer, LoadBalancer,
LoadBalancerProvider}
+import
org.apache.openwhisk.standalone.StandaloneDockerSupport.{checkOrAllocatePort,
containerName, createRunCmd}
+
+import scala.concurrent.{ExecutionContext, Future}
+import scala.reflect.io.Directory
+import scala.util.Try
+
+class KafkaLauncher(docker: StandaloneDockerClient,
+kafkaPort: Int,
+kafkaDockerPort: Int,
+zkPort: Int,
+workDir: File,
+kafkaUi: Boolean)(implicit logging: Logging,
+ ec: ExecutionContext,
+ actorSystem: ActorSystem,
+ materializer: ActorMaterializer,
+ tid: TransactionId) {
+
+ def run(): Future[Seq[ServiceContainer]] = {
+for {
+ kafkaSvcs <- runKafka()
+ uiSvcs <- if (kafkaUi) runKafkaUI() else
Future.successful(Seq.empty[ServiceContainer])
+} yield kafkaSvcs ++ uiSvcs
+ }
+
+ def runKafka(): Future[Seq[ServiceContainer]] = {
+
+//Below setting based on
https://rmoff.net/2018/08/02/kafka-listeners-explained/
+// We configure two listeners where one is used for host based application
and other is used for docker based application
+// to connect to Kafka server running on host
Review comment:
It would be helpful to add some more detail - for example, controller /
invoker will use `LISTENER_LOCAL` since they run in the same JVM as the
embedded Kafka and Kafka UI will run in a Docker container and use
`LISTENER_DOCKER`.
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_r326150664
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -64,100 +64,165 @@ 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
+
+ /** Pithy descriptor for logging. */
+ def name: String
+
+ /** Does message indicate slot is free? */
+ def isSlotFree: Option[InvokerInstanceId]
+
+ /** Does message contain a result? */
+ def result: Option[Either[ActivationId, WhiskActivation]]
+
+ /**
+ * Is the acknowledgement for an activation that failed internally?
+ * For some message, this is not relevant and the result is None.
+ */
+ def isSystemError: Option[Boolean]
+
+ def activationId: ActivationId
+
+ /** Serializes the message to JSON. */
+ def toJson: JsValue
+
+ /**
+ * Converts the message to a more compact form if it cannot cross the
message bus as is or some of its details are not necessary.
+ */
+ def shrink: AcknowledegmentMessage
}
/**
- * 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 in situtations when
the resource slot and the action
Review comment:
that's not a nit - that's a typo :D
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 #4628: Embedded Kafka support in OpenWhisk Standalone mode
sven-lange-last commented on a change in pull request #4628: Embedded Kafka
support in OpenWhisk Standalone mode
URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326150432
##
File path:
core/standalone/src/main/scala/org/apache/openwhisk/standalone/StandaloneOpenWhisk.scala
##
@@ -63,6 +64,25 @@ class Conf(arguments: Seq[String]) extends
ScallopConf(arguments) {
val apiGwPort = opt[Int](descr = "Api Gateway Port", default = Some(3234),
noshort = true)
val dataDir = opt[File](descr = "Directory used for storage", default =
Some(StandaloneOpenWhisk.defaultWorkDir))
+ val kafka = opt[Boolean](descr = "Enable embedded Kafka support", noshort =
true)
+ val kafkaUi = opt[Boolean](descr = "Enable Kafka UI", noshort = true)
+
+ val kafkaPort = opt[Int](
+descr = "Kafka port. If not specified then 9092 or some random free port
(if 9092 is busy) would be used",
+noshort = true,
+required = false)
+
+ val kafkaDockerPort = opt[Int](
+descr = "Kafka port for use by docker based services. If not specified
then 9091 or some random free port " +
+ "(if 9091 is busy) would be used",
+noshort = true,
+required = false)
Review comment:
Would it make sense to add a `default = Some(9091)` here?
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 #4628: Embedded Kafka support in OpenWhisk Standalone mode
sven-lange-last commented on a change in pull request #4628: Embedded Kafka
support in OpenWhisk Standalone mode
URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326150517
##
File path:
core/standalone/src/main/scala/org/apache/openwhisk/standalone/StandaloneOpenWhisk.scala
##
@@ -63,6 +64,25 @@ class Conf(arguments: Seq[String]) extends
ScallopConf(arguments) {
val apiGwPort = opt[Int](descr = "Api Gateway Port", default = Some(3234),
noshort = true)
val dataDir = opt[File](descr = "Directory used for storage", default =
Some(StandaloneOpenWhisk.defaultWorkDir))
+ val kafka = opt[Boolean](descr = "Enable embedded Kafka support", noshort =
true)
+ val kafkaUi = opt[Boolean](descr = "Enable Kafka UI", noshort = true)
+
+ val kafkaPort = opt[Int](
+descr = "Kafka port. If not specified then 9092 or some random free port
(if 9092 is busy) would be used",
+noshort = true,
+required = false)
+
+ val kafkaDockerPort = opt[Int](
+descr = "Kafka port for use by docker based services. If not specified
then 9091 or some random free port " +
+ "(if 9091 is busy) would be used",
+noshort = true,
+required = false)
+
+ val zkPort = opt[Int](
+descr = "Zookeeper port. If not specified then 2181 or some random free
port (if 2181 is busy) would be used",
+noshort = true,
+required = false)
Review comment:
Would it make sense to add a `default = Some(2181)` here?
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 #4628: Embedded Kafka support in OpenWhisk Standalone mode
sven-lange-last commented on a change in pull request #4628: Embedded Kafka
support in OpenWhisk Standalone mode
URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326150351
##
File path:
core/standalone/src/main/scala/org/apache/openwhisk/standalone/StandaloneOpenWhisk.scala
##
@@ -63,6 +64,25 @@ class Conf(arguments: Seq[String]) extends
ScallopConf(arguments) {
val apiGwPort = opt[Int](descr = "Api Gateway Port", default = Some(3234),
noshort = true)
val dataDir = opt[File](descr = "Directory used for storage", default =
Some(StandaloneOpenWhisk.defaultWorkDir))
+ val kafka = opt[Boolean](descr = "Enable embedded Kafka support", noshort =
true)
+ val kafkaUi = opt[Boolean](descr = "Enable Kafka UI", noshort = true)
+
+ val kafkaPort = opt[Int](
+descr = "Kafka port. If not specified then 9092 or some random free port
(if 9092 is busy) would be used",
+noshort = true,
+required = false)
Review comment:
Would it make sense to add a `default = Some(9092)` here?
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 #4628: Embedded Kafka support in OpenWhisk Standalone mode
sven-lange-last commented on a change in pull request #4628: Embedded Kafka
support in OpenWhisk Standalone mode
URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326155928
##
File path:
core/standalone/src/main/scala/org/apache/openwhisk/standalone/KafkaLauncher.scala
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.standalone
+
+import java.io.File
+
+import akka.actor.ActorSystem
+import akka.stream.ActorMaterializer
+import kafka.server.KafkaConfig
+import net.manub.embeddedkafka.{EmbeddedKafka, EmbeddedKafkaConfig}
+import org.apache.commons.io.FileUtils
+import org.apache.openwhisk.common.{Logging, TransactionId}
+import org.apache.openwhisk.core.WhiskConfig
+import org.apache.openwhisk.core.WhiskConfig.kafkaHosts
+import org.apache.openwhisk.core.entity.ControllerInstanceId
+import org.apache.openwhisk.core.loadBalancer.{LeanBalancer, LoadBalancer,
LoadBalancerProvider}
+import
org.apache.openwhisk.standalone.StandaloneDockerSupport.{checkOrAllocatePort,
containerName, createRunCmd}
+
+import scala.concurrent.{ExecutionContext, Future}
+import scala.reflect.io.Directory
+import scala.util.Try
+
+class KafkaLauncher(docker: StandaloneDockerClient,
+kafkaPort: Int,
+kafkaDockerPort: Int,
+zkPort: Int,
+workDir: File,
+kafkaUi: Boolean)(implicit logging: Logging,
+ ec: ExecutionContext,
+ actorSystem: ActorSystem,
+ materializer: ActorMaterializer,
+ tid: TransactionId) {
+
+ def run(): Future[Seq[ServiceContainer]] = {
+for {
+ kafkaSvcs <- runKafka()
+ uiSvcs <- if (kafkaUi) runKafkaUI() else
Future.successful(Seq.empty[ServiceContainer])
+} yield kafkaSvcs ++ uiSvcs
+ }
+
+ def runKafka(): Future[Seq[ServiceContainer]] = {
+
+//Below setting based on
https://rmoff.net/2018/08/02/kafka-listeners-explained/
+// We configure two listeners where one is used for host based application
and other is used for docker based application
+// to connect to Kafka server running on host
+val brokerProps = Map(
+ KafkaConfig.ListenersProp ->
s"LISTENER_LOCAL://localhost:$kafkaPort,LISTENER_DOCKER://localhost:$kafkaDockerPort",
+ KafkaConfig.AdvertisedListenersProp ->
s"LISTENER_LOCAL://localhost:$kafkaPort,LISTENER_DOCKER://${StandaloneDockerSupport
+.getLocalHostIp()}:$kafkaDockerPort",
+ KafkaConfig.ListenerSecurityProtocolMapProp ->
"LISTENER_LOCAL:PLAINTEXT,LISTENER_DOCKER:PLAINTEXT",
+ KafkaConfig.InterBrokerListenerNameProp -> "LISTENER_LOCAL")
+implicit val config: EmbeddedKafkaConfig =
+ EmbeddedKafkaConfig(kafkaPort = kafkaPort, zooKeeperPort = zkPort,
customBrokerProperties = brokerProps)
+
+val t = Try {
+ EmbeddedKafka.startZooKeeper(createDir("zookeeper"))
+ EmbeddedKafka.startKafka(createDir("kafka"))
+}
+
+Future
+ .fromTry(t)
+ .map(
+_ =>
+ Seq(
+ServiceContainer(kafkaPort, s"localhost:$kafkaPort", "kafka"),
+ServiceContainer(
+ kafkaDockerPort,
+ s"${StandaloneDockerSupport.getLocalHostIp()}:$kafkaDockerPort",
+ "kafka-docker"),
+ServiceContainer(zkPort, "Zookeeper", "zookeeper")))
+ }
+
+ def runKafkaUI(): Future[Seq[ServiceContainer]] = {
+val hostIp = StandaloneDockerSupport.getLocalHostIp()
+val port = checkOrAllocatePort(9000)
+val env = Map(
+ "ZOOKEEPER_CONNECT" -> s"$hostIp:$zkPort",
+ "KAFKA_BROKERCONNECT" -> s"$hostIp:$kafkaDockerPort",
+ "JVM_OPTS" -> "-Xms32M -Xmx64M",
+ "SERVER_SERVLET_CONTEXTPATH" -> "/")
+
+logging.info(this, s"Starting Kafka Drop UI port: $port")
+val name = containerName("kafka-drop-ui")
+val params = Map("-p" -> Set(s"$port:9000"))
+val args = createRunCmd(name, env, params)
+
+val f = docker.runDetached("obsidiandynamics/kafdrop", args, true)
+f.map(_ => Seq(ServiceContainer(port, s"http://localhost:$port";, name
[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_r326154802
##
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala
##
@@ -54,9 +54,10 @@ object InvokerReactive extends InvokerProvider {
* @param Boolean is true iff the activation was a blocking request
* @param ControllerInstanceId the originating controller/loadbalancer id
* @param UUID is the UUID for the namespace owning the activation
- * @param Boolean is true this is resource free message and false if this is
a result forwarding message
+ * @param AcknowledegmentMessage the acknowledgement message to send
*/
- type ActiveAck = (TransactionId, WhiskActivation, Boolean,
ControllerInstanceId, UUID, Boolean) => Future[Any]
+ type ActiveAck =
Review comment:
echo rant.
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_r326154490
##
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
##
@@ -628,8 +637,15 @@ class ContainerProxy(
// completion message which frees a load balancer slot is sent after the
active ack future
// completes to ensure proper ordering.
val sendResult = if (job.msg.blocking) {
- activation.map(
-sendActiveAck(tid, _, job.msg.blocking, job.msg.rootControllerIndex,
job.msg.user.namespace.uuid, false))
+ activation.map { result =>
+sendActiveAck(
+ tid,
+ result,
+ job.msg.blocking,
+ job.msg.rootControllerIndex,
+ job.msg.user.namespace.uuid,
+ ResultMessage(tid, result))
Review comment:
good idea - I won't do that here, as you suggest, can be done subsequently.
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_r326156558
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -64,100 +64,165 @@ 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
+
+ /** Pithy descriptor for logging. */
+ def name: String
+
+ /** Does message indicate slot is free? */
+ def isSlotFree: Option[InvokerInstanceId]
+
+ /** Does message contain a result? */
+ def result: Option[Either[ActivationId, WhiskActivation]]
+
+ /**
+ * Is the acknowledgement for an activation that failed internally?
+ * For some message, this is not relevant and the result is None.
+ */
+ def isSystemError: Option[Boolean]
+
+ def activationId: ActivationId
+
+ /** Serializes the message to JSON. */
+ def toJson: JsValue
+
+ /**
+ * Converts the message to a more compact form if it cannot cross the
message bus as is or some of its details are not necessary.
+ */
+ def shrink: AcknowledegmentMessage
}
/**
- * 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 in situtations when
the resource slot and the action
+ * result are available at the same time, and so the split-phase notification
is not necessary. Instead the message
+ * combines the `CompletionMessage` and `ResultMessage`. The `response` may be
an `ActivationId` to allow for failures
+ * to send the activation result because of event-bus size limitations.
*/
-case class CompletionMessage(override val transid: TransactionId,
- activationId: ActivationId,
- isSystemError: Boolean,
- invoker: InvokerInstanceId)
+case class CombinedCompletionAndResultMessage(override val transid:
TransactionId,
+ response: Either[ActivationId,
WhiskActivation],
+ override val isSystemError:
Option[Boolean],
+ invoker: InvokerInstanceId)
extends AcknowledegmentMessage(transid) {
-
- override def toString = {
-activationId.asString
- }
+ override def name = "combined"
+ override def result = Some(response)
+ override def isSlotFree = Some(invoker)
+ override def activationId = response.fold(identity, _.activationId)
+ override def toJson = CombinedCompletionAndResultMessage.serdes.write(this)
+ override def shrink = copy(response = response.flatMap(a =>
Left(a.activationId)))
+ override def toString = response.fold(identity, _.activationId).asString
}
-object CompletionMessage extends DefaultJsonProtocol {
- def parse(msg: String): Try[CompletionMessage] =
Try(serdes.read(msg.parseJson))
- implicit val serdes = jsonFormat4(CompletionMessage.apply)
+/**
+ * This message is sent from an invoker to the controller, once the resource
slot in the invoker (used by the
+ * corresponding activation) free again (i.e., after log collection). The
`CompletionMessage` is part of a split
+ * phase notification to the load balancer where an invoker first sends a
`ResultMessage` and later sends the
+ * `CompletionMessage`.
+ */
+case class CompletionMessage(override val transid: TransactionId,
+ override val activationId: ActivationId,
+ override val isSystemError: Option[Boolean],
+ invoker: InvokerInstanceId)
+extends AcknowledegmentMessage(transid) {
+ override def name = "completion"
+ override def result = None
+ override def isSlotFree = Some(invoker)
+ override def toJson = CompletionMessage.serdes.write(this)
+ override def shrink = this
+ override def toString = activationId.asString
}
/**
- * That message will be sent from the invoker to the controller after action
completion if the user wants to have
- * the result immediately (blocking activation).
-
[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_r326158858
##
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala
##
@@ -145,43 +146,34 @@ class InvokerReactive(
new MessageFeed("activation", logging, consumer, maxPeek, 1.second,
processActivationMessage)
})
- /** Sends an active-ack. */
private val ack: InvokerReactive.ActiveAck = (tid: TransactionId,
activationResult:
WhiskActivation,
blockingInvoke: Boolean,
controllerInstance:
ControllerInstanceId,
userId: UUID,
-isSlotFree: Boolean) => {
+acknowledegment:
AcknowledegmentMessage) => {
Review comment:
yes - i considered doing that but because the current types allow for an
`Either`, whereas all the call sites do know the activation, it would allow for
a weakening should someone construct an ack with a Left where a Right is
expected. I don't think there's a way to restrict the constructors (given they
are case classes) without adding more types. So I think the current signature
is tighter.
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_r326152274
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -64,100 +64,165 @@ 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
+
+ /** Pithy descriptor for logging. */
+ def name: String
+
+ /** Does message indicate slot is free? */
+ def isSlotFree: Option[InvokerInstanceId]
+
+ /** Does message contain a result? */
+ def result: Option[Either[ActivationId, WhiskActivation]]
+
+ /**
+ * Is the acknowledgement for an activation that failed internally?
+ * For some message, this is not relevant and the result is None.
+ */
+ def isSystemError: Option[Boolean]
+
+ def activationId: ActivationId
+
+ /** Serializes the message to JSON. */
+ def toJson: JsValue
+
+ /**
+ * Converts the message to a more compact form if it cannot cross the
message bus as is or some of its details are not necessary.
+ */
+ def shrink: AcknowledegmentMessage
}
/**
- * 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 in situtations when
the resource slot and the action
+ * result are available at the same time, and so the split-phase notification
is not necessary. Instead the message
+ * combines the `CompletionMessage` and `ResultMessage`. The `response` may be
an `ActivationId` to allow for failures
+ * to send the activation result because of event-bus size limitations.
*/
-case class CompletionMessage(override val transid: TransactionId,
- activationId: ActivationId,
- isSystemError: Boolean,
- invoker: InvokerInstanceId)
+case class CombinedCompletionAndResultMessage(override val transid:
TransactionId,
+ response: Either[ActivationId,
WhiskActivation],
+ override val isSystemError:
Option[Boolean],
+ invoker: InvokerInstanceId)
extends AcknowledegmentMessage(transid) {
-
- override def toString = {
-activationId.asString
- }
+ override def name = "combined"
+ override def result = Some(response)
+ override def isSlotFree = Some(invoker)
+ override def activationId = response.fold(identity, _.activationId)
+ override def toJson = CombinedCompletionAndResultMessage.serdes.write(this)
+ override def shrink = copy(response = response.flatMap(a =>
Left(a.activationId)))
+ override def toString = response.fold(identity, _.activationId).asString
}
-object CompletionMessage extends DefaultJsonProtocol {
- def parse(msg: String): Try[CompletionMessage] =
Try(serdes.read(msg.parseJson))
- implicit val serdes = jsonFormat4(CompletionMessage.apply)
+/**
+ * This message is sent from an invoker to the controller, once the resource
slot in the invoker (used by the
+ * corresponding activation) free again (i.e., after log collection). The
`CompletionMessage` is part of a split
+ * phase notification to the load balancer where an invoker first sends a
`ResultMessage` and later sends the
+ * `CompletionMessage`.
+ */
+case class CompletionMessage(override val transid: TransactionId,
+ override val activationId: ActivationId,
+ override val isSystemError: Option[Boolean],
+ invoker: InvokerInstanceId)
+extends AcknowledegmentMessage(transid) {
+ override def name = "completion"
+ override def result = None
+ override def isSlotFree = Some(invoker)
+ override def toJson = CompletionMessage.serdes.write(this)
+ override def shrink = this
+ override def toString = activationId.asString
}
/**
- * That message will be sent from the invoker to the controller after action
completion if the user wants to have
- * the result immediately (blocking activation).
-
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode
chetanmeh commented on a change in pull request #4628: Embedded Kafka support
in OpenWhisk Standalone mode
URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326184627
##
File path:
core/standalone/src/main/scala/org/apache/openwhisk/standalone/KafkaLauncher.scala
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.standalone
+
+import java.io.File
+
+import akka.actor.ActorSystem
+import akka.stream.ActorMaterializer
+import kafka.server.KafkaConfig
+import net.manub.embeddedkafka.{EmbeddedKafka, EmbeddedKafkaConfig}
+import org.apache.commons.io.FileUtils
+import org.apache.openwhisk.common.{Logging, TransactionId}
+import org.apache.openwhisk.core.WhiskConfig
+import org.apache.openwhisk.core.WhiskConfig.kafkaHosts
+import org.apache.openwhisk.core.entity.ControllerInstanceId
+import org.apache.openwhisk.core.loadBalancer.{LeanBalancer, LoadBalancer,
LoadBalancerProvider}
+import
org.apache.openwhisk.standalone.StandaloneDockerSupport.{checkOrAllocatePort,
containerName, createRunCmd}
+
+import scala.concurrent.{ExecutionContext, Future}
+import scala.reflect.io.Directory
+import scala.util.Try
+
+class KafkaLauncher(docker: StandaloneDockerClient,
+kafkaPort: Int,
+kafkaDockerPort: Int,
+zkPort: Int,
+workDir: File,
+kafkaUi: Boolean)(implicit logging: Logging,
+ ec: ExecutionContext,
+ actorSystem: ActorSystem,
+ materializer: ActorMaterializer,
+ tid: TransactionId) {
+
+ def run(): Future[Seq[ServiceContainer]] = {
+for {
+ kafkaSvcs <- runKafka()
+ uiSvcs <- if (kafkaUi) runKafkaUI() else
Future.successful(Seq.empty[ServiceContainer])
+} yield kafkaSvcs ++ uiSvcs
+ }
+
+ def runKafka(): Future[Seq[ServiceContainer]] = {
+
+//Below setting based on
https://rmoff.net/2018/08/02/kafka-listeners-explained/
+// We configure two listeners where one is used for host based application
and other is used for docker based application
+// to connect to Kafka server running on host
Review comment:
👍 Done
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode
chetanmeh commented on a change in pull request #4628: Embedded Kafka support
in OpenWhisk Standalone mode
URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326186606
##
File path:
core/standalone/src/main/scala/org/apache/openwhisk/standalone/StandaloneOpenWhisk.scala
##
@@ -63,6 +64,25 @@ class Conf(arguments: Seq[String]) extends
ScallopConf(arguments) {
val apiGwPort = opt[Int](descr = "Api Gateway Port", default = Some(3234),
noshort = true)
val dataDir = opt[File](descr = "Directory used for storage", default =
Some(StandaloneOpenWhisk.defaultWorkDir))
+ val kafka = opt[Boolean](descr = "Enable embedded Kafka support", noshort =
true)
+ val kafkaUi = opt[Boolean](descr = "Enable Kafka UI", noshort = true)
+
+ val kafkaPort = opt[Int](
+descr = "Kafka port. If not specified then 9092 or some random free port
(if 9092 is busy) would be used",
+noshort = true,
+required = false)
Review comment:
This is done on purpose. Port 9092 is more like a "preferred" port. If no
explicit option is provided then system would try to start Kafka by default at
9092. However if that port is busy then a random port would be selected.
This is done to ensure that Standalone OpenWhisk needs minimum required free
port (for now only 3233). Rest all services can be started at any port and then
dependent service would be configured as per randomly selected port
If a user explicitly provides a port like `--kafka-port 9010` then Kafka
would be started at that port. If the port is busy then Kafka would fail to
start. This support is mostly meant for test which would like to know where
Kafka would start
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 #4628: Embedded Kafka support in OpenWhisk Standalone mode
sven-lange-last commented on a change in pull request #4628: Embedded Kafka
support in OpenWhisk Standalone mode
URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326189887
##
File path:
core/standalone/src/main/scala/org/apache/openwhisk/standalone/StandaloneOpenWhisk.scala
##
@@ -393,6 +417,42 @@ object StandaloneOpenWhisk extends SLF4JLogging {
Await.result(g, 5.minutes)
}
+ private def startKafka(workDir: File, dockerClient: StandaloneDockerClient,
conf: Conf, kafkaUi: Boolean)(
+implicit logging: Logging,
+as: ActorSystem,
+ec: ExecutionContext,
+materializer: ActorMaterializer): (Int, Seq[ServiceContainer]) = {
+val kafkaPort = getPort(conf.kafkaPort.toOption, 9092)
Review comment:
If we add a default value to the `kafkaPort` option, can probably solve this
differently and not repeat the port number literal here?
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 #4628: Embedded Kafka support in OpenWhisk Standalone mode
sven-lange-last commented on a change in pull request #4628: Embedded Kafka
support in OpenWhisk Standalone mode
URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326191971
##
File path:
core/standalone/src/main/scala/org/apache/openwhisk/standalone/StandaloneOpenWhisk.scala
##
@@ -63,6 +64,25 @@ class Conf(arguments: Seq[String]) extends
ScallopConf(arguments) {
val apiGwPort = opt[Int](descr = "Api Gateway Port", default = Some(3234),
noshort = true)
val dataDir = opt[File](descr = "Directory used for storage", default =
Some(StandaloneOpenWhisk.defaultWorkDir))
+ val kafka = opt[Boolean](descr = "Enable embedded Kafka support", noshort =
true)
+ val kafkaUi = opt[Boolean](descr = "Enable Kafka UI", noshort = true)
+
+ val kafkaPort = opt[Int](
+descr = "Kafka port. If not specified then 9092 or some random free port
(if 9092 is busy) would be used",
+noshort = true,
+required = false)
Review comment:
I understand. Background of my proposal was that there is some duplication
of the default ports. We have the port number 9092 for Kafka here AND in
`startKafka()`. Wouldn't it make sense to define a `val defaultKafkaPort =
9092` and use it to build the description string and in `startKafka()`?
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 commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode
chetanmeh commented on a change in pull request #4628: Embedded Kafka support
in OpenWhisk Standalone mode
URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326193031
##
File path:
core/standalone/src/main/scala/org/apache/openwhisk/standalone/KafkaLauncher.scala
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.standalone
+
+import java.io.File
+
+import akka.actor.ActorSystem
+import akka.stream.ActorMaterializer
+import kafka.server.KafkaConfig
+import net.manub.embeddedkafka.{EmbeddedKafka, EmbeddedKafkaConfig}
+import org.apache.commons.io.FileUtils
+import org.apache.openwhisk.common.{Logging, TransactionId}
+import org.apache.openwhisk.core.WhiskConfig
+import org.apache.openwhisk.core.WhiskConfig.kafkaHosts
+import org.apache.openwhisk.core.entity.ControllerInstanceId
+import org.apache.openwhisk.core.loadBalancer.{LeanBalancer, LoadBalancer,
LoadBalancerProvider}
+import
org.apache.openwhisk.standalone.StandaloneDockerSupport.{checkOrAllocatePort,
containerName, createRunCmd}
+
+import scala.concurrent.{ExecutionContext, Future}
+import scala.reflect.io.Directory
+import scala.util.Try
+
+class KafkaLauncher(docker: StandaloneDockerClient,
+kafkaPort: Int,
+kafkaDockerPort: Int,
+zkPort: Int,
+workDir: File,
+kafkaUi: Boolean)(implicit logging: Logging,
+ ec: ExecutionContext,
+ actorSystem: ActorSystem,
+ materializer: ActorMaterializer,
+ tid: TransactionId) {
+
+ def run(): Future[Seq[ServiceContainer]] = {
+for {
+ kafkaSvcs <- runKafka()
+ uiSvcs <- if (kafkaUi) runKafkaUI() else
Future.successful(Seq.empty[ServiceContainer])
+} yield kafkaSvcs ++ uiSvcs
+ }
+
+ def runKafka(): Future[Seq[ServiceContainer]] = {
+
+//Below setting based on
https://rmoff.net/2018/08/02/kafka-listeners-explained/
+// We configure two listeners where one is used for host based application
and other is used for docker based application
+// to connect to Kafka server running on host
+val brokerProps = Map(
+ KafkaConfig.ListenersProp ->
s"LISTENER_LOCAL://localhost:$kafkaPort,LISTENER_DOCKER://localhost:$kafkaDockerPort",
+ KafkaConfig.AdvertisedListenersProp ->
s"LISTENER_LOCAL://localhost:$kafkaPort,LISTENER_DOCKER://${StandaloneDockerSupport
+.getLocalHostIp()}:$kafkaDockerPort",
+ KafkaConfig.ListenerSecurityProtocolMapProp ->
"LISTENER_LOCAL:PLAINTEXT,LISTENER_DOCKER:PLAINTEXT",
+ KafkaConfig.InterBrokerListenerNameProp -> "LISTENER_LOCAL")
+implicit val config: EmbeddedKafkaConfig =
+ EmbeddedKafkaConfig(kafkaPort = kafkaPort, zooKeeperPort = zkPort,
customBrokerProperties = brokerProps)
+
+val t = Try {
+ EmbeddedKafka.startZooKeeper(createDir("zookeeper"))
+ EmbeddedKafka.startKafka(createDir("kafka"))
+}
+
+Future
+ .fromTry(t)
+ .map(
+_ =>
+ Seq(
+ServiceContainer(kafkaPort, s"localhost:$kafkaPort", "kafka"),
+ServiceContainer(
+ kafkaDockerPort,
+ s"${StandaloneDockerSupport.getLocalHostIp()}:$kafkaDockerPort",
+ "kafka-docker"),
+ServiceContainer(zkPort, "Zookeeper", "zookeeper")))
+ }
+
+ def runKafkaUI(): Future[Seq[ServiceContainer]] = {
+val hostIp = StandaloneDockerSupport.getLocalHostIp()
+val port = checkOrAllocatePort(9000)
+val env = Map(
+ "ZOOKEEPER_CONNECT" -> s"$hostIp:$zkPort",
+ "KAFKA_BROKERCONNECT" -> s"$hostIp:$kafkaDockerPort",
+ "JVM_OPTS" -> "-Xms32M -Xmx64M",
+ "SERVER_SERVLET_CONTEXTPATH" -> "/")
+
+logging.info(this, s"Starting Kafka Drop UI port: $port")
+val name = containerName("kafka-drop-ui")
+val params = Map("-p" -> Set(s"$port:9000"))
+val args = createRunCmd(name, env, params)
+
+val f = docker.runDetached("obsidiandynamics/kafdrop", args, true)
+f.map(_ => Seq(ServiceContainer(port, s"http://localhost:$port";, name)))
+
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode
chetanmeh commented on a change in pull request #4628: Embedded Kafka support
in OpenWhisk Standalone mode
URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326198147
##
File path:
core/standalone/src/main/scala/org/apache/openwhisk/standalone/StandaloneOpenWhisk.scala
##
@@ -63,6 +64,25 @@ class Conf(arguments: Seq[String]) extends
ScallopConf(arguments) {
val apiGwPort = opt[Int](descr = "Api Gateway Port", default = Some(3234),
noshort = true)
val dataDir = opt[File](descr = "Directory used for storage", default =
Some(StandaloneOpenWhisk.defaultWorkDir))
+ val kafka = opt[Boolean](descr = "Enable embedded Kafka support", noshort =
true)
+ val kafkaUi = opt[Boolean](descr = "Enable Kafka UI", noshort = true)
+
+ val kafkaPort = opt[Int](
+descr = "Kafka port. If not specified then 9092 or some random free port
(if 9092 is busy) would be used",
+noshort = true,
+required = false)
Review comment:
👍 Done that now
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 commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode
chetanmeh commented on a change in pull request #4628: Embedded Kafka support
in OpenWhisk Standalone mode
URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326198456
##
File path:
core/standalone/src/main/scala/org/apache/openwhisk/standalone/StandaloneOpenWhisk.scala
##
@@ -393,6 +417,42 @@ object StandaloneOpenWhisk extends SLF4JLogging {
Await.result(g, 5.minutes)
}
+ private def startKafka(workDir: File, dockerClient: StandaloneDockerClient,
conf: Conf, kafkaUi: Boolean)(
+implicit logging: Logging,
+as: ActorSystem,
+ec: ExecutionContext,
+materializer: ActorMaterializer): (Int, Seq[ServiceContainer]) = {
+val kafkaPort = getPort(conf.kafkaPort.toOption, 9092)
Review comment:
Ack. Done as you suggested by moving defaults to constant and reusing that
across various places
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 issue #4635: Combine active ack and slot release for setups not collecting logs directly
chetanmeh opened a new issue #4635: Combine active ack and slot release for setups not collecting logs directly URL: https://github.com/apache/openwhisk/issues/4635 This is extension of #4624 as discussed in [comments][1]. We can optimize the Kafka flow for setups which do not collect logs as part of invocation flow and instead use some sort of `LogDriverLogStore` Such setups can send `CombinedCompletionAndResultMessage` if 1. `job.action.limits.logs.asMegaBytes == 0.MB` 2. OR `LogDriverLogStore` is being used [1]: https://github.com/apache/openwhisk/pull/4624#discussion_r325996077 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 commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh 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_r326202534
##
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
##
@@ -628,8 +637,15 @@ class ContainerProxy(
// completion message which frees a load balancer slot is sent after the
active ack future
// completes to ensure proper ordering.
val sendResult = if (job.msg.blocking) {
- activation.map(
-sendActiveAck(tid, _, job.msg.blocking, job.msg.rootControllerIndex,
job.msg.user.namespace.uuid, false))
+ activation.map { result =>
+sendActiveAck(
+ tid,
+ result,
+ job.msg.blocking,
+ job.msg.rootControllerIndex,
+ job.msg.user.namespace.uuid,
+ ResultMessage(tid, result))
Review comment:
Opened #4635 to track 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 #4628: Embedded Kafka support in OpenWhisk Standalone mode
sven-lange-last commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326203622 ## File path: tests/src/test/scala/org/apache/openwhisk/standalone/StandaloneServerFixture.scala ## @@ -93,6 +95,7 @@ trait StandaloneServerFixture extends TestSuite with BeforeAndAfterAll with Stre manifestFile.foreach(FileUtils.deleteQuietly) serverProcess.destroy() } +FileUtils.forceDelete(new File(dataDirPath)) Review comment: I guess this answers my question from above regarding data deletion when the process exits... 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_r326203452
##
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala
##
@@ -145,43 +146,38 @@ class InvokerReactive(
new MessageFeed("activation", logging, consumer, maxPeek, 1.second,
processActivationMessage)
})
- /** Sends an active-ack. */
private val ack: InvokerReactive.ActiveAck = (tid: TransactionId,
activationResult:
WhiskActivation,
blockingInvoke: Boolean,
controllerInstance:
ControllerInstanceId,
userId: UUID,
-isSlotFree: Boolean) => {
+acknowledegment:
AcknowledegmentMessage) => {
implicit val transid: TransactionId = tid
-def send(res: Either[ActivationId, WhiskActivation], recovery: Boolean =
false) = {
- val msg = if (isSlotFree) {
-val aid = res.fold(identity, _.activationId)
-val isWhiskSystemError = res.fold(_ => false, _.response.isWhiskError)
-CompletionMessage(transid, aid, isWhiskSystemError, instance)
- } else {
-ResultMessage(transid, res)
- }
-
+def send(msg: AcknowledegmentMessage, recovery: Boolean = false) = {
producer.send(topic = "completed" + controllerInstance.asString,
msg).andThen {
case Success(_) =>
- logging.info(
-this,
-s"posted ${if (recovery) "recovery" else "completion"} of
activation ${activationResult.activationId}")
+ val info = if (recovery) s"recovery ${msg.messageType}" else
msg.messageType
+ logging.info(this, s"posted $info of activation
${activationResult.activationId}")
}
}
// UserMetrics are sent, when the slot is free again. This ensures, that
all metrics are sent.
-if (UserEvents.enabled && isSlotFree) {
- EventMessage.from(activationResult, s"invoker${instance.instance}",
userId) match {
-case Success(msg) => UserEvents.send(producer, msg)
-case Failure(t) => logging.error(this, s"activation event was not
sent: $t")
+if (UserEvents.enabled && acknowledegment.isSlotFree.nonEmpty) {
+ acknowledegment.result.foreach {
+case Right(activationResult) =>
+ EventMessage.from(activationResult, s"invoker${instance.instance}",
userId) match {
+case Success(msg) => UserEvents.send(producer, msg)
+case Failure(t) => logging.error(this, s"activation event was
not sent: $t")
+ }
+case _ => // all acknowledegment messages should have a result
}
Review comment:
@sven-lange-last this is an alternate implementation but does not remove the
`activation` parameter which turns out to be more extensive refactoring. We can
leave this change in place and remove the parameter as part of addressing
@chetanmeh's comment about turning this function into a trait implementation.
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_r326201849
##
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala
##
@@ -54,9 +54,10 @@ object InvokerReactive extends InvokerProvider {
* @param Boolean is true iff the activation was a blocking request
* @param ControllerInstanceId the originating controller/loadbalancer id
* @param UUID is the UUID for the namespace owning the activation
- * @param Boolean is true this is resource free message and false if this is
a result forwarding message
+ * @param AcknowledegmentMessage the acknowledgement message to send
*/
- type ActiveAck = (TransactionId, WhiskActivation, Boolean,
ControllerInstanceId, UUID, Boolean) => Future[Any]
+ type ActiveAck =
Review comment:
This ends up being a bit more extensive than at first appearances so we
should defer it to a subsequent patch.
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_r326204785
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -64,100 +64,165 @@ 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
+
+ /** Pithy descriptor for logging. */
+ def name: String
+
+ /** Does message indicate slot is free? */
+ def isSlotFree: Option[InvokerInstanceId]
+
+ /** Does message contain a result? */
+ def result: Option[Either[ActivationId, WhiskActivation]]
+
+ /**
+ * Is the acknowledgement for an activation that failed internally?
+ * For some message, this is not relevant and the result is None.
+ */
+ def isSystemError: Option[Boolean]
+
+ def activationId: ActivationId
+
+ /** Serializes the message to JSON. */
+ def toJson: JsValue
+
+ /**
+ * Converts the message to a more compact form if it cannot cross the
message bus as is or some of its details are not necessary.
+ */
+ def shrink: AcknowledegmentMessage
}
/**
- * 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 in situtations when
the resource slot and the action
+ * result are available at the same time, and so the split-phase notification
is not necessary. Instead the message
+ * combines the `CompletionMessage` and `ResultMessage`. The `response` may be
an `ActivationId` to allow for failures
+ * to send the activation result because of event-bus size limitations.
*/
-case class CompletionMessage(override val transid: TransactionId,
- activationId: ActivationId,
- isSystemError: Boolean,
- invoker: InvokerInstanceId)
+case class CombinedCompletionAndResultMessage(override val transid:
TransactionId,
+ response: Either[ActivationId,
WhiskActivation],
+ override val isSystemError:
Option[Boolean],
+ invoker: InvokerInstanceId)
extends AcknowledegmentMessage(transid) {
-
- override def toString = {
-activationId.asString
- }
+ override def name = "combined"
+ override def result = Some(response)
+ override def isSlotFree = Some(invoker)
+ override def activationId = response.fold(identity, _.activationId)
+ override def toJson = CombinedCompletionAndResultMessage.serdes.write(this)
+ override def shrink = copy(response = response.flatMap(a =>
Left(a.activationId)))
+ override def toString = response.fold(identity, _.activationId).asString
}
-object CompletionMessage extends DefaultJsonProtocol {
- def parse(msg: String): Try[CompletionMessage] =
Try(serdes.read(msg.parseJson))
- implicit val serdes = jsonFormat4(CompletionMessage.apply)
+/**
+ * This message is sent from an invoker to the controller, once the resource
slot in the invoker (used by the
+ * corresponding activation) free again (i.e., after log collection). The
`CompletionMessage` is part of a split
+ * phase notification to the load balancer where an invoker first sends a
`ResultMessage` and later sends the
+ * `CompletionMessage`.
+ */
+case class CompletionMessage(override val transid: TransactionId,
+ override val activationId: ActivationId,
+ override val isSystemError: Option[Boolean],
+ invoker: InvokerInstanceId)
+extends AcknowledegmentMessage(transid) {
+ override def name = "completion"
+ override def result = None
+ override def isSlotFree = Some(invoker)
+ override def toJson = CompletionMessage.serdes.write(this)
+ override def shrink = this
+ override def toString = activationId.asString
}
/**
- * That message will be sent from the invoker to the controller after action
completion if the user wants to have
- * the result immediately (blocking activation).
-
[GitHub] [openwhisk] chetanmeh commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh 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_r326205999
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -64,100 +64,165 @@ 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
+
+ /** Pithy descriptor for logging. */
+ def name: String
+
+ /** Does message indicate slot is free? */
+ def isSlotFree: Option[InvokerInstanceId]
+
+ /** Does message contain a result? */
+ def result: Option[Either[ActivationId, WhiskActivation]]
+
+ /**
+ * Is the acknowledgement for an activation that failed internally?
+ * For some message, this is not relevant and the result is None.
+ */
+ def isSystemError: Option[Boolean]
+
+ def activationId: ActivationId
+
+ /** Serializes the message to JSON. */
+ def toJson: JsValue
+
+ /**
+ * Converts the message to a more compact form if it cannot cross the
message bus as is or some of its details are not necessary.
+ */
+ def shrink: AcknowledegmentMessage
}
/**
- * 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 in situtations when
the resource slot and the action
+ * result are available at the same time, and so the split-phase notification
is not necessary. Instead the message
+ * combines the `CompletionMessage` and `ResultMessage`. The `response` may be
an `ActivationId` to allow for failures
+ * to send the activation result because of event-bus size limitations.
*/
-case class CompletionMessage(override val transid: TransactionId,
- activationId: ActivationId,
- isSystemError: Boolean,
- invoker: InvokerInstanceId)
+case class CombinedCompletionAndResultMessage(override val transid:
TransactionId,
+ response: Either[ActivationId,
WhiskActivation],
+ override val isSystemError:
Option[Boolean],
+ invoker: InvokerInstanceId)
extends AcknowledegmentMessage(transid) {
-
- override def toString = {
-activationId.asString
- }
+ override def name = "combined"
+ override def result = Some(response)
+ override def isSlotFree = Some(invoker)
+ override def activationId = response.fold(identity, _.activationId)
+ override def toJson = CombinedCompletionAndResultMessage.serdes.write(this)
+ override def shrink = copy(response = response.flatMap(a =>
Left(a.activationId)))
+ override def toString = response.fold(identity, _.activationId).asString
}
-object CompletionMessage extends DefaultJsonProtocol {
- def parse(msg: String): Try[CompletionMessage] =
Try(serdes.read(msg.parseJson))
- implicit val serdes = jsonFormat4(CompletionMessage.apply)
+/**
+ * This message is sent from an invoker to the controller, once the resource
slot in the invoker (used by the
+ * corresponding activation) free again (i.e., after log collection). The
`CompletionMessage` is part of a split
+ * phase notification to the load balancer where an invoker first sends a
`ResultMessage` and later sends the
+ * `CompletionMessage`.
+ */
+case class CompletionMessage(override val transid: TransactionId,
+ override val activationId: ActivationId,
+ override val isSystemError: Option[Boolean],
+ invoker: InvokerInstanceId)
+extends AcknowledegmentMessage(transid) {
+ override def name = "completion"
+ override def result = None
+ override def isSlotFree = Some(invoker)
+ override def toJson = CompletionMessage.serdes.write(this)
+ override def shrink = this
+ override def toString = activationId.asString
}
/**
- * That message will be sent from the invoker to the controller after action
completion if the user wants to have
- * the result immediately (blocking activation).
[GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode
sven-lange-last commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326209107 ## File path: tests/src/test/scala/org/apache/openwhisk/standalone/StandaloneServerFixture.scala ## @@ -93,6 +95,7 @@ trait StandaloneServerFixture extends TestSuite with BeforeAndAfterAll with Stre manifestFile.foreach(FileUtils.deleteQuietly) serverProcess.destroy() } +FileUtils.forceDelete(new File(dataDirPath)) Review comment: At least for 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] codecov-io edited a comment on issue #4628: Embedded Kafka support in OpenWhisk Standalone mode
codecov-io edited a comment 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&el=h1) Report > Merging [#4628](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/06362ad83608335a01ccbb4ef449ec4c0e70dd99?src=pr&el=desc) will **increase** coverage by `35.46%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#4628 +/- ## === + Coverage 43.31% 78.77% +35.46% === Files 183 183 Lines8305 8305 Branches 574 573-1 === + Hits 3597 6542 +2945 + Misses 4708 1763 -2945 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [...openwhisk/common/tracing/OpenTracingProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvbW1vbi90cmFjaW5nL09wZW5UcmFjaW5nUHJvdmlkZXIuc2NhbGE=) | `21.15% <0%> (+1.92%)` | :arrow_up: | | [...re/database/MultipleReadersSingleWriterCache.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvTXVsdGlwbGVSZWFkZXJzU2luZ2xlV3JpdGVyQ2FjaGUuc2NhbGE=) | `98% <0%> (+2%)` | :arrow_up: | | [...apache/openwhisk/core/entitlement/Collection.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXRsZW1lbnQvQ29sbGVjdGlvbi5zY2FsYQ==) | `87.5% <0%> (+2.5%)` | :arrow_up: | | [.../org/apache/openwhisk/http/PoolingRestClient.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2h0dHAvUG9vbGluZ1Jlc3RDbGllbnQuc2NhbGE=) | `91.17% <0%> (+2.94%)` | :arrow_up: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `4% <0%> (+4%)` | :arrow_up: | | [...tainerpool/docker/DockerClientWithFileAccess.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9kb2NrZXIvRG9ja2VyQ2xpZW50V2l0aEZpbGVBY2Nlc3Muc2NhbGE=) | `96% <0%> (+4%)` | :arrow_up: | | [...whisk/connector/kafka/KafkaConsumerConnector.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2Nvbm5lY3Rvci9rYWZrYS9LYWZrYUNvbnN1bWVyQ29ubmVjdG9yLnNjYWxh) | `57.74% <0%> (+4.22%)` | :arrow_up: | | [...enwhisk/core/loadBalancer/InvokerSupervision.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0ludm9rZXJTdXBlcnZpc2lvbi5zY2FsYQ==) | `95.77% <0%> (+4.92%)` | :arrow_up: | | [...pache/openwhisk/core/entity/ConcurrencyLimit.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0NvbmN1cnJlbmN5TGltaXQuc2NhbGE=) | `94.11% <0%> (+5.88%)` | :arrow_up: | | [...la/org/apache/openwhisk/core/entity/LogLimit.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0xvZ0xpbWl0LnNjYWxh) | `94.11% <0%> (+5.88%)` | :arrow_up: | | ... and [122 more](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=footer). Last update [06362ad...03b99e4](https://codecov.io/gh/apache/openwhisk/pull/4628?src=
[GitHub] [openwhisk] codecov-io edited a comment on issue #4628: Embedded Kafka support in OpenWhisk Standalone mode
codecov-io edited a comment 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&el=h1) Report > Merging [#4628](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/06362ad83608335a01ccbb4ef449ec4c0e70dd99?src=pr&el=desc) will **increase** coverage by `35.54%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#4628 +/- ## === + Coverage 43.31% 78.85% +35.54% === Files 183 183 Lines8305 8305 Branches 574 573-1 === + Hits 3597 6549 +2952 + Misses 4708 1756 -2952 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [...openwhisk/common/tracing/OpenTracingProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvbW1vbi90cmFjaW5nL09wZW5UcmFjaW5nUHJvdmlkZXIuc2NhbGE=) | `21.15% <0%> (+1.92%)` | :arrow_up: | | [...re/database/MultipleReadersSingleWriterCache.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvTXVsdGlwbGVSZWFkZXJzU2luZ2xlV3JpdGVyQ2FjaGUuc2NhbGE=) | `98% <0%> (+2%)` | :arrow_up: | | [...apache/openwhisk/core/entitlement/Collection.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXRsZW1lbnQvQ29sbGVjdGlvbi5zY2FsYQ==) | `87.5% <0%> (+2.5%)` | :arrow_up: | | [.../org/apache/openwhisk/http/PoolingRestClient.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2h0dHAvUG9vbGluZ1Jlc3RDbGllbnQuc2NhbGE=) | `91.17% <0%> (+2.94%)` | :arrow_up: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `4% <0%> (+4%)` | :arrow_up: | | [...tainerpool/docker/DockerClientWithFileAccess.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9kb2NrZXIvRG9ja2VyQ2xpZW50V2l0aEZpbGVBY2Nlc3Muc2NhbGE=) | `96% <0%> (+4%)` | :arrow_up: | | [...whisk/connector/kafka/KafkaConsumerConnector.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2Nvbm5lY3Rvci9rYWZrYS9LYWZrYUNvbnN1bWVyQ29ubmVjdG9yLnNjYWxh) | `57.74% <0%> (+4.22%)` | :arrow_up: | | [...enwhisk/core/loadBalancer/InvokerSupervision.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0ludm9rZXJTdXBlcnZpc2lvbi5zY2FsYQ==) | `95.77% <0%> (+4.92%)` | :arrow_up: | | [...pache/openwhisk/core/entity/ConcurrencyLimit.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0NvbmN1cnJlbmN5TGltaXQuc2NhbGE=) | `94.11% <0%> (+5.88%)` | :arrow_up: | | [...la/org/apache/openwhisk/core/entity/LogLimit.scala](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0xvZ0xpbWl0LnNjYWxh) | `94.11% <0%> (+5.88%)` | :arrow_up: | | ... and [123 more](https://codecov.io/gh/apache/openwhisk/pull/4628/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4628?src=pr&el=footer). Last update [06362ad...03b99e4](https://codecov.io/gh/apache/openwhisk/pull/4628?src=
[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_r326236622
##
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala
##
@@ -145,43 +146,38 @@ class InvokerReactive(
new MessageFeed("activation", logging, consumer, maxPeek, 1.second,
processActivationMessage)
})
- /** Sends an active-ack. */
private val ack: InvokerReactive.ActiveAck = (tid: TransactionId,
activationResult:
WhiskActivation,
blockingInvoke: Boolean,
controllerInstance:
ControllerInstanceId,
userId: UUID,
-isSlotFree: Boolean) => {
+acknowledegment:
AcknowledegmentMessage) => {
implicit val transid: TransactionId = tid
-def send(res: Either[ActivationId, WhiskActivation], recovery: Boolean =
false) = {
- val msg = if (isSlotFree) {
-val aid = res.fold(identity, _.activationId)
-val isWhiskSystemError = res.fold(_ => false, _.response.isWhiskError)
-CompletionMessage(transid, aid, isWhiskSystemError, instance)
- } else {
-ResultMessage(transid, res)
- }
-
+def send(msg: AcknowledegmentMessage, recovery: Boolean = false) = {
producer.send(topic = "completed" + controllerInstance.asString,
msg).andThen {
case Success(_) =>
- logging.info(
-this,
-s"posted ${if (recovery) "recovery" else "completion"} of
activation ${activationResult.activationId}")
+ val info = if (recovery) s"recovery ${msg.messageType}" else
msg.messageType
+ logging.info(this, s"posted $info of activation
${activationResult.activationId}")
}
}
// UserMetrics are sent, when the slot is free again. This ensures, that
all metrics are sent.
-if (UserEvents.enabled && isSlotFree) {
- EventMessage.from(activationResult, s"invoker${instance.instance}",
userId) match {
-case Success(msg) => UserEvents.send(producer, msg)
-case Failure(t) => logging.error(this, s"activation event was not
sent: $t")
+if (UserEvents.enabled && acknowledegment.isSlotFree.nonEmpty) {
+ acknowledegment.result.foreach {
+case Right(activationResult) =>
+ EventMessage.from(activationResult, s"invoker${instance.instance}",
userId) match {
+case Success(msg) => UserEvents.send(producer, msg)
+case Failure(t) => logging.error(this, s"activation event was
not sent: $t")
+ }
+case _ => // all acknowledegment messages should have a result
}
Review comment:
Thanks a lot. What do you think about adding a `logging.error()` to the
`case _ =>` path - just to make sure that we have a chance to catch the
unexpected case if somebody makes a mistake with setting up
`AcknowledegmentMessage`?
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 commented on issue #4628: Embedded Kafka support in OpenWhisk Standalone mode
chetanmeh commented on issue #4628: Embedded Kafka support in OpenWhisk Standalone mode URL: https://github.com/apache/openwhisk/pull/4628#issuecomment-533189190 Thanks Sven for the 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] chetanmeh merged pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode
chetanmeh merged pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode URL: https://github.com/apache/openwhisk/pull/4628 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_r326256172
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
##
@@ -64,100 +64,165 @@ 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
+
+ /** Pithy descriptor for logging. */
+ def name: String
+
+ /** Does message indicate slot is free? */
+ def isSlotFree: Option[InvokerInstanceId]
+
+ /** Does message contain a result? */
+ def result: Option[Either[ActivationId, WhiskActivation]]
+
+ /**
+ * Is the acknowledgement for an activation that failed internally?
+ * For some message, this is not relevant and the result is None.
+ */
+ def isSystemError: Option[Boolean]
+
+ def activationId: ActivationId
+
+ /** Serializes the message to JSON. */
+ def toJson: JsValue
+
+ /**
+ * Converts the message to a more compact form if it cannot cross the
message bus as is or some of its details are not necessary.
+ */
+ def shrink: AcknowledegmentMessage
}
/**
- * 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 in situtations when
the resource slot and the action
+ * result are available at the same time, and so the split-phase notification
is not necessary. Instead the message
+ * combines the `CompletionMessage` and `ResultMessage`. The `response` may be
an `ActivationId` to allow for failures
+ * to send the activation result because of event-bus size limitations.
*/
-case class CompletionMessage(override val transid: TransactionId,
- activationId: ActivationId,
- isSystemError: Boolean,
- invoker: InvokerInstanceId)
+case class CombinedCompletionAndResultMessage(override val transid:
TransactionId,
+ response: Either[ActivationId,
WhiskActivation],
+ override val isSystemError:
Option[Boolean],
+ invoker: InvokerInstanceId)
extends AcknowledegmentMessage(transid) {
-
- override def toString = {
-activationId.asString
- }
+ override def name = "combined"
+ override def result = Some(response)
+ override def isSlotFree = Some(invoker)
+ override def activationId = response.fold(identity, _.activationId)
+ override def toJson = CombinedCompletionAndResultMessage.serdes.write(this)
+ override def shrink = copy(response = response.flatMap(a =>
Left(a.activationId)))
+ override def toString = response.fold(identity, _.activationId).asString
}
-object CompletionMessage extends DefaultJsonProtocol {
- def parse(msg: String): Try[CompletionMessage] =
Try(serdes.read(msg.parseJson))
- implicit val serdes = jsonFormat4(CompletionMessage.apply)
+/**
+ * This message is sent from an invoker to the controller, once the resource
slot in the invoker (used by the
+ * corresponding activation) free again (i.e., after log collection). The
`CompletionMessage` is part of a split
+ * phase notification to the load balancer where an invoker first sends a
`ResultMessage` and later sends the
+ * `CompletionMessage`.
+ */
+case class CompletionMessage(override val transid: TransactionId,
+ override val activationId: ActivationId,
+ override val isSystemError: Option[Boolean],
+ invoker: InvokerInstanceId)
+extends AcknowledegmentMessage(transid) {
+ override def name = "completion"
+ override def result = None
+ override def isSlotFree = Some(invoker)
+ override def toJson = CompletionMessage.serdes.write(this)
+ override def shrink = this
+ override def toString = activationId.asString
}
/**
- * That message will be sent from the invoker to the controller after action
completion if the user wants to have
- * the result immediately (blocking activation).
-
[GitHub] [openwhisk] rabbah opened a new issue #4636: Refactor active ack method signature to be an interface
rabbah opened a new issue #4636: Refactor active ack method signature to be an interface URL: https://github.com/apache/openwhisk/issues/4636 In core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala: > */ - type ActiveAck = (TransactionId, WhiskActivation, Boolean, ControllerInstanceId, UUID, Boolean) => Future[Any] + type ActiveAck = Minor observation (or Rant!) - I always struggle in IDE whenever I want to see impl of parameters which are specified as function signature instead of trait (like in ActiveAck). Here things are bit simpler as we specify a type alias which narrows down the search. Otherwise one need to check the whole call hierarchy to understand where is. the actual impl Having a trait enables easier checking of possible implementations to understand the code flow. This is pre existing stuff ... but may be later we refactor it and use a proper trait for such critical flows. From @chetanmeh in #4624. 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 #4617: Add kind "unknown" to fallback activations
rabbah commented on a change in pull request #4617: Add kind "unknown" to
fallback activations
URL: https://github.com/apache/openwhisk/pull/4617#discussion_r326259380
##
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/entity/Exec.scala
##
@@ -242,6 +242,9 @@ object Exec extends ArgNormalizer[Exec] with
DefaultJsonProtocol {
protected[core] val SEQUENCE = "sequence"
protected[core] val BLACKBOX = "blackbox"
+ // This is for error cases while cannot get the `kind` of Exec
Review comment:
```suggestion
// This is for error cases when the action `kind` may not be known.
```
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 merged pull request #4617: Add kind "unknown" to fallback activations
rabbah merged pull request #4617: Add kind "unknown" to fallback activations URL: https://github.com/apache/openwhisk/pull/4617 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 #4624: Combines active ack and slot release when both are available.
codecov-io edited a comment on issue #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#issuecomment-531316564 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=h1) Report > Merging [#4624](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/400a7915115576a363858788a6d080c389a80317?src=pr&el=desc) will **decrease** coverage by `28.24%`. > The diff coverage is `79.12%`. [](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#4624 +/- ## === - Coverage 84.44% 56.19% -28.25% === Files 183 183 Lines8306 8344 +38 Branches 572 575+3 === - Hits 7014 4689 -2325 - Misses 1292 3655 +2363 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [.../scala/org/apache/openwhisk/core/entity/Exec.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0V4ZWMuc2NhbGE=) | `82.81% <ø> (-7.82%)` | :arrow_down: | | [...enwhisk/core/loadBalancer/CommonLoadBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0NvbW1vbkxvYWRCYWxhbmNlci5zY2FsYQ==) | `81.08% <100%> (-0.17%)` | :arrow_down: | | [.../openwhisk/core/containerpool/ContainerProxy.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJQcm94eS5zY2FsYQ==) | `85.39% <100%> (-7.39%)` | :arrow_down: | | [...pache/openwhisk/core/invoker/InvokerReactive.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyUmVhY3RpdmUuc2NhbGE=) | `76.22% <50%> (-3.78%)` | :arrow_down: | | [.../org/apache/openwhisk/core/connector/Message.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29ubmVjdG9yL01lc3NhZ2Uuc2NhbGE=) | `51.96% <86.27%> (-12.91%)` | :arrow_down: | | [...base/cosmosdb/cache/WhiskChangeEventObserver.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL1doaXNrQ2hhbmdlRXZlbnRPYnNlcnZlci5zY2FsYQ==) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...nwhisk/core/database/cosmosdb/CosmosDBConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJDb25maWcuc2NhbGE=) | `0% <0%> (-100%)` | :arrow_down: | | [...hisk/core/database/cosmosdb/ReferenceCounted.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUmVmZXJlbmNlQ291bnRlZC5zY2FsYQ==) | `0% <0%> (-100%)` | :arrow_down: | | [...re/database/cosmosdb/CollectionResourceUsage.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29sbGVjdGlvblJlc291cmNlVXNhZ2Uuc2NhbGE=) | `0% <0%> (-100%)` | :arrow_down: | | ... and [118 more](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=footer). Last update [400a791...
[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&el=h1) Report > Merging [#4627](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/e9dd2073cd2343545d006cd71ca01dc24114c216?src=pr&el=desc) will **increase** coverage by `35.89%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master#4627 +/- ## === + Coverage 42.92% 78.81% +35.89% === Files 183 183 Lines8305 8323 +18 Branches 573 574+1 === + Hits 3565 6560 +2995 + Misses 4740 1763 -2977 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [...openwhisk/common/tracing/OpenTracingProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvbW1vbi90cmFjaW5nL09wZW5UcmFjaW5nUHJvdmlkZXIuc2NhbGE=) | `21.15% <0%> (+1.92%)` | :arrow_up: | | [...re/database/MultipleReadersSingleWriterCache.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvTXVsdGlwbGVSZWFkZXJzU2luZ2xlV3JpdGVyQ2FjaGUuc2NhbGE=) | `98% <0%> (+2%)` | :arrow_up: | | [...apache/openwhisk/core/entitlement/Collection.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXRsZW1lbnQvQ29sbGVjdGlvbi5zY2FsYQ==) | `87.5% <0%> (+2.5%)` | :arrow_up: | | [.../org/apache/openwhisk/http/PoolingRestClient.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2h0dHAvUG9vbGluZ1Jlc3RDbGllbnQuc2NhbGE=) | `91.17% <0%> (+2.94%)` | :arrow_up: | | [...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlUHJvdmlkZXIuc2NhbGE=) | `4% <0%> (+4%)` | :arrow_up: | | [...whisk/connector/kafka/KafkaConsumerConnector.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2Nvbm5lY3Rvci9rYWZrYS9LYWZrYUNvbnN1bWVyQ29ubmVjdG9yLnNjYWxh) | `57.74% <0%> (+4.22%)` | :arrow_up: | | [...enwhisk/core/loadBalancer/InvokerSupervision.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0ludm9rZXJTdXBlcnZpc2lvbi5zY2FsYQ==) | `95.77% <0%> (+4.92%)` | :arrow_up: | | [...pache/openwhisk/core/entity/ConcurrencyLimit.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0NvbmN1cnJlbmN5TGltaXQuc2NhbGE=) | `94.11% <0%> (+5.88%)` | :arrow_up: | | [...la/org/apache/openwhisk/core/entity/LogLimit.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0xvZ0xpbWl0LnNjYWxh) | `94.11% <0%> (+5.88%)` | :arrow_up: | | [...org/apache/openwhisk/core/entity/MemoryLimit.scala](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L01lbW9yeUxpbWl0LnNjYWxh) | `93.75% <0%> (+6.25%)` | :arrow_up: | | ... and [123 more](https://codecov.io/gh/apache/openwhisk/pull/4627/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4627?src=pr&el=footer). Last update [e9dd207...b298b34](https://codecov.io/gh/apache/openwhisk/pull/4627?src=p
[GitHub] [openwhisk] rabbah commented on issue #4624: Combines active ack and slot release when both are available.
rabbah commented on issue #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#issuecomment-533322879 @chetanmeh I refactored the active acker's signature/type. @sven-lange-last we can't remove `WhiskActivation` - the tests in container proxy rely on it being available and removing it causes a number of issues in testing the proxy (specifically, the tests use non-blocking activations which only generate CompletionMessages and as a result there is no Activation to check as the tests do now.) 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 #4624: Combines active ack and slot release when both are available.
rabbah commented on issue #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#issuecomment-533322961 I removed WIP label as I think all comments are addressed at this point. 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 #4624: Combines active ack and slot release when both are available.
codecov-io edited a comment on issue #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#issuecomment-531316564 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=h1) Report > Merging [#4624](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/400a7915115576a363858788a6d080c389a80317?src=pr&el=desc) will **decrease** coverage by `5.7%`. > The diff coverage is `79.34%`. [](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#4624 +/- ## == - Coverage 84.44% 78.74% -5.71% == Files 183 183 Lines8306 8345 +39 Branches 572 572 == - Hits 7014 6571 -443 - Misses 1292 1774 +482 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [.../scala/org/apache/openwhisk/core/entity/Exec.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0V4ZWMuc2NhbGE=) | `90.62% <ø> (ø)` | :arrow_up: | | [...enwhisk/core/loadBalancer/CommonLoadBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0NvbW1vbkxvYWRCYWxhbmNlci5zY2FsYQ==) | `81.08% <100%> (-0.17%)` | :arrow_down: | | [.../openwhisk/core/containerpool/ContainerProxy.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJQcm94eS5zY2FsYQ==) | `92.88% <100%> (+0.1%)` | :arrow_up: | | [...pache/openwhisk/core/invoker/InvokerReactive.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyUmVhY3RpdmUuc2NhbGE=) | `75.4% <50%> (-4.6%)` | :arrow_down: | | [.../org/apache/openwhisk/core/connector/Message.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29ubmVjdG9yL01lc3NhZ2Uuc2NhbGE=) | `72.81% <86.53%> (+7.95%)` | :arrow_up: | | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.89%)` | :arrow_down: | | [...tabase/cosmosdb/cache/CacheInvalidatorConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3JDb25maWcuc2NhbGE=) | `0% <0%> (-94.74%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | ... and [12 more](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=footer). Last update [400a79
[GitHub] [openwhisk] codecov-io edited a comment on issue #4624: Combines active ack and slot release when both are available.
codecov-io edited a comment on issue #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#issuecomment-531316564 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=h1) Report > Merging [#4624](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/400a7915115576a363858788a6d080c389a80317?src=pr&el=desc) will **decrease** coverage by `5.7%`. > The diff coverage is `79.34%`. [](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#4624 +/- ## == - Coverage 84.44% 78.74% -5.71% == Files 183 183 Lines8306 8345 +39 Branches 572 572 == - Hits 7014 6571 -443 - Misses 1292 1774 +482 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [.../scala/org/apache/openwhisk/core/entity/Exec.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0V4ZWMuc2NhbGE=) | `90.62% <ø> (ø)` | :arrow_up: | | [...enwhisk/core/loadBalancer/CommonLoadBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0NvbW1vbkxvYWRCYWxhbmNlci5zY2FsYQ==) | `81.08% <100%> (-0.17%)` | :arrow_down: | | [.../openwhisk/core/containerpool/ContainerProxy.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJQcm94eS5zY2FsYQ==) | `92.88% <100%> (+0.1%)` | :arrow_up: | | [...pache/openwhisk/core/invoker/InvokerReactive.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyUmVhY3RpdmUuc2NhbGE=) | `75.4% <50%> (-4.6%)` | :arrow_down: | | [.../org/apache/openwhisk/core/connector/Message.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29ubmVjdG9yL01lc3NhZ2Uuc2NhbGE=) | `72.81% <86.53%> (+7.95%)` | :arrow_up: | | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.89%)` | :arrow_down: | | [...tabase/cosmosdb/cache/CacheInvalidatorConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3JDb25maWcuc2NhbGE=) | `0% <0%> (-94.74%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | ... and [12 more](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=footer). Last update [400a79
[GitHub] [openwhisk] codecov-io edited a comment on issue #4624: Combines active ack and slot release when both are available.
codecov-io edited a comment on issue #4624: Combines active ack and slot release when both are available. URL: https://github.com/apache/openwhisk/pull/4624#issuecomment-531316564 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=h1) Report > Merging [#4624](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/400a7915115576a363858788a6d080c389a80317?src=pr&el=desc) will **decrease** coverage by `5.67%`. > The diff coverage is `80%`. [](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#4624 +/- ## == - Coverage 84.44% 78.76% -5.68% == Files 183 183 Lines8306 8346 +40 Branches 572 571 -1 == - Hits 7014 6574 -440 - Misses 1292 1772 +480 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [.../scala/org/apache/openwhisk/core/entity/Exec.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0V4ZWMuc2NhbGE=) | `90.62% <ø> (ø)` | :arrow_up: | | [...enwhisk/core/loadBalancer/CommonLoadBalancer.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvbG9hZEJhbGFuY2VyL0NvbW1vbkxvYWRCYWxhbmNlci5zY2FsYQ==) | `81.08% <100%> (-0.17%)` | :arrow_down: | | [.../openwhisk/core/containerpool/ContainerProxy.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXJQcm94eS5zY2FsYQ==) | `92.88% <100%> (+0.1%)` | :arrow_up: | | [...pache/openwhisk/core/invoker/InvokerReactive.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvaW52b2tlci9JbnZva2VyUmVhY3RpdmUuc2NhbGE=) | `75.6% <51.85%> (-4.4%)` | :arrow_down: | | [.../org/apache/openwhisk/core/connector/Message.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29ubmVjdG9yL01lc3NhZ2Uuc2NhbGE=) | `73.78% <88.46%> (+8.92%)` | :arrow_up: | | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.89%)` | :arrow_down: | | [...tabase/cosmosdb/cache/CacheInvalidatorConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3JDb25maWcuc2NhbGE=) | `0% <0%> (-94.74%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | ... and [13 more](https://codecov.io/gh/apache/openwhisk/pull/4624/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4624?src=pr&el=footer). Last update [400a7
[GitHub] [openwhisk] rabbah commented on a change in pull request #4627: Add descriptions how to update, remove or rename runtime kinds and language families
rabbah 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_r326432228 ## 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 preexisting 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. + +Obviously, 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. Clearly, the steps below should be spaced out in time 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. Review comment: ```suggestion 1. Deprecate the runtime kind by setting `"deprecated": true` in the [runtimes manifest](actions-new.md#the-runtimes-manifest). This setting prevents new actions from being created with the deprecated action kind. In addition, existing actions cannot be changed to the deprecated action kind any more. ``` 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 #4627: Add descriptions how to update, remove or rename runtime kinds and language families
rabbah 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_r326431818 ## 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 preexisting actions. + +### Updating runtimes + +Follow these steps to update a particular runtime kind: + +1. Update the runtimes' container image. Review comment: ```suggestion 1. Update the runtime's container image. ``` 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 #4627: Add descriptions how to update, remove or rename runtime kinds and language families
rabbah 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_r326431621 ## 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. Review comment: ```suggestion * Remove languages that are no longer needed. ``` 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 #4627: Add descriptions how to update, remove or rename runtime kinds and language families
rabbah 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_r326432316 ## 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 preexisting 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. + +Obviously, 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. Clearly, the steps below should be spaced out in time 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. Review comment: ```suggestion 2. Ask owners of existing actions affected by the runtime kind to remove or update their actions to a different action kind. ``` 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 #4627: Add descriptions how to update, remove or rename runtime kinds and language families
rabbah 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_r326432044 ## 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 preexisting 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. + +Obviously, 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: ```suggestion Obviously, this approach should only be used if existing actions do not break with the new container image. If a new container image may break existing actions, consider introducing a new runtime kind instead. ``` 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 #4627: Add descriptions how to update, remove or rename runtime kinds and language families
rabbah 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_r326431689 ## 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: Review comment: ```suggestion Over time, you may need to do one or more of the following: ``` 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 #4627: Add descriptions how to update, remove or rename runtime kinds and language families
rabbah 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_r326432413 ## 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 preexisting 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. + +Obviously, 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. Clearly, the steps below should be spaced out in time 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. Review comment: @sven-lange-last if you're done this in the past, do you plan to share the scripts? 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 #4627: Add descriptions how to update, remove or rename runtime kinds and language families
rabbah 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_r326431572 ## 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. Review comment: ```suggestion * Remove runtime versions that are no longer supported - for example, Node.js 6. ``` 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 #4627: Add descriptions how to update, remove or rename runtime kinds and language families
rabbah 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_r326432711 ## 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 preexisting 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. + +Obviously, 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. Clearly, the steps below should be spaced out in time 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 not contain actions with the runtime kind to be removed, remove the runtime kind from the [runtimes manifest](actions-new.md#the-runtimes-manifest). +5. 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 longer be read, updated, deleted or invoked. Review comment: ```suggestion 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 longer be read, updated, deleted or invoked. ``` 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 #4628: Embedded Kafka support in OpenWhisk Standalone mode
rabbah commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326433208 ## File path: core/standalone/README.md ## @@ -204,7 +219,35 @@ Api Gateway mode can be enabled via `--api-gw` flag. In this mode upon launch a would be launched on port `3234` (can be changed with `--api-gw-port`). In this mode you can make use of the [api gateway][4] support. + Using Kafka + +Standalone OpenWhisk supports launching an [embedded kafka][5]. This mode is mostly useful for developers working on OpenWhisk +implementation itself. + +``` +java -jar openwhisk-standalone.jar --kafka +``` + +It also supports launching a Kafka UI based on [Kafdrop 3][6] which enables seeing the topics created and structure of messages +exchanged on those topics. + +``` +java -jar openwhisk-standalone.jar --kafka --kafka-ui +``` + +By default the ui server would be accessible at http://localhost:9000. In case 9000 port is busy then a random port would +be selected. TO find out the port look for message in log like below (or grep for `whisk-kafka-drop-ui`) Review comment: TO -> To 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 #4628: Embedded Kafka support in OpenWhisk Standalone mode
rabbah commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326433077 ## File path: core/standalone/README.md ## @@ -204,7 +219,35 @@ Api Gateway mode can be enabled via `--api-gw` flag. In this mode upon launch a would be launched on port `3234` (can be changed with `--api-gw-port`). In this mode you can make use of the [api gateway][4] support. + Using Kafka + +Standalone OpenWhisk supports launching an [embedded kafka][5]. This mode is mostly useful for developers working on OpenWhisk +implementation itself. + +``` +java -jar openwhisk-standalone.jar --kafka +``` + +It also supports launching a Kafka UI based on [Kafdrop 3][6] which enables seeing the topics created and structure of messages Review comment: `[Kafdrop 3][6]` -> is this supposed to link to two different places, "3" and "6"? 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 opened a new issue #4637: inefficiencies and issues with the actions CRUD API
rabbah opened a new issue #4637: inefficiencies and issues with the actions CRUD API URL: https://github.com/apache/openwhisk/issues/4637 Actions are represented by the type `WhiskAction`, which includes a property of type `Exec`. The `Exec` instances may include the code inline. The REST API allows the controller to ignore the `Exec` value for the most part, but the current implementation has parallel types `WhiskActionMetadata` and `ExecMetadata` to explicitly drop the `Exec` values. There is a lot of redundancy in the code - and the two types have drifted and changes are in one type and not the other. There is at least one bug that I'm aware of because of the drift. Thanks to @chetanmeh's work, we do have uniform support for attaching "code" as separate entities (CouchDB attachments, S3, etc.). The implementation also allows for inlined code up to a certain limit. Because of these changes, we can drop the parallel types and improve the CRUD API's performance. 1. a GET of an action via the REST api today returns a butchered `exec` field with no code... is it OK to actually return the internal representation of the "code" attachment or do we continue to hide it? 1. is it ok to cache inlined code (since they are soft references and can be gced from the cache)? 1. all runtimes should be using attachments in the runtimes manifest, and so we should remove the property for "attachment" from the runtimes manifest 1. action operations continue to fetch the attachment and inline it (... only to discard it later for the "metadata" classes), this is unnecessary overhead 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 #4609: Add namespace field to activation log
rabbah commented on issue #4609: Add namespace field to activation log URL: https://github.com/apache/openwhisk/pull/4609#issuecomment-533362102 Take a look at `makeSequenceActivation` in `SequenceActions`. 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 commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode
chetanmeh commented on a change in pull request #4628: Embedded Kafka support in OpenWhisk Standalone mode URL: https://github.com/apache/openwhisk/pull/4628#discussion_r326465642 ## File path: core/standalone/README.md ## @@ -204,7 +219,35 @@ Api Gateway mode can be enabled via `--api-gw` flag. In this mode upon launch a would be launched on port `3234` (can be changed with `--api-gw-port`). In this mode you can make use of the [api gateway][4] support. + Using Kafka + +Standalone OpenWhisk supports launching an [embedded kafka][5]. This mode is mostly useful for developers working on OpenWhisk +implementation itself. + +``` +java -jar openwhisk-standalone.jar --kafka +``` + +It also supports launching a Kafka UI based on [Kafdrop 3][6] which enables seeing the topics created and structure of messages Review comment: No thats the actual name [Kafdrop 3](https://github.com/obsidiandynamics/kafdrop) 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 commented on a change in pull request #4624: Combines active ack and slot release when both are available.
chetanmeh 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_r326466033
##
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala
##
@@ -49,14 +49,21 @@ object InvokerReactive extends InvokerProvider {
* are either completion messages for an activation to indicate a resource
slot is free, or result-forwarding
* messages for continuations (e.g., sequences and conductor actions).
*
- * @param TransactionId the transaction id for the activation
- * @param WhiskActivaiton is the activation result
- * @param Boolean is true iff the activation was a blocking request
- * @param ControllerInstanceId the originating controller/loadbalancer id
- * @param UUID is the UUID for the namespace owning the activation
- * @param Boolean is true this is resource free message and false if this is
a result forwarding message
+ * @param tid the transaction id for the activation
+ * @param activaiton is the activation result
+ * @param blockingInvoke is true iff the activation was a blocking request
+ * @param controllerInstance the originating controller/loadbalancer id
+ * @param userId is the UUID for the namespace owning the activation
+ * @param acknowledegment the acknowledgement message to send
*/
- type ActiveAck = (TransactionId, WhiskActivation, Boolean,
ControllerInstanceId, UUID, Boolean) => Future[Any]
+ trait ActiveAck {
+def apply(tid: TransactionId,
Review comment:
Nice use of `apply`! Minimizes the impact of switch
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 commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images
chetanmeh 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_r326467823 ## File path: ansible/group_vars/all ## @@ -55,7 +55,8 @@ whisk: # configuration parameters related to support runtimes (see org.apache.openwhisk.core.entity.ExecManifest for schema of the manifest). # briefly the parameters are: # -# runtimes_registry: optional registry (with trailing slack) where to pull docker images from for runtimes and backbox images +# runtimes_registry: optional registry (with trailing slash) where to pull docker images from for default runtimes (in manifest) +# user_images_registry: optional registry (with trailing slash) where to pull docker images from for blackbox images Review comment: Just to confirm wrt compatibility - If user was earlier configuring `runtimes_registry` which was used for "both" images from for runtimes and backbox images would now have to configure both `runtimes_registry` and `user_images_registry`. Or system would default to `runtimes_registry` if `user_images_registry` is not configured 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 #4632: Activation Persister Service
codecov-io commented on issue #4632: Activation Persister Service URL: https://github.com/apache/openwhisk/pull/4632#issuecomment-533411344 # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4632?src=pr&el=h1) Report > Merging [#4632](https://codecov.io/gh/apache/openwhisk/pull/4632?src=pr&el=desc) into [master](https://codecov.io/gh/apache/openwhisk/commit/400a7915115576a363858788a6d080c389a80317?src=pr&el=desc) will **decrease** coverage by `5.58%`. > The diff coverage is `100%`. [](https://codecov.io/gh/apache/openwhisk/pull/4632?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#4632 +/- ## == - Coverage 84.44% 78.85% -5.59% == Files 183 183 Lines8306 8306 Branches 572 572 == - Hits 7014 6550 -464 - Misses 1292 1756 +464 ``` | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/4632?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [...la/org/apache/openwhisk/http/BasicRasService.scala](https://codecov.io/gh/apache/openwhisk/pull/4632/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2h0dHAvQmFzaWNSYXNTZXJ2aWNlLnNjYWxh) | `100% <100%> (ø)` | :arrow_up: | | [...core/database/cosmosdb/RxObservableImplicits.scala](https://codecov.io/gh/apache/openwhisk/pull/4632/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvUnhPYnNlcnZhYmxlSW1wbGljaXRzLnNjYWxh) | `0% <0%> (-100%)` | :arrow_down: | | [...ore/database/cosmosdb/cache/CacheInvalidator.scala](https://codecov.io/gh/apache/openwhisk/pull/4632/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3Iuc2NhbGE=) | `0% <0%> (-100%)` | :arrow_down: | | [...core/database/cosmosdb/CosmosDBArtifactStore.scala](https://codecov.io/gh/apache/openwhisk/pull/4632/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJBcnRpZmFjdFN0b3JlLnNjYWxh) | `0% <0%> (-95.89%)` | :arrow_down: | | [...tabase/cosmosdb/cache/CacheInvalidatorConfig.scala](https://codecov.io/gh/apache/openwhisk/pull/4632/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NhY2hlSW52YWxpZGF0b3JDb25maWcuc2NhbGE=) | `0% <0%> (-94.74%)` | :arrow_down: | | [...sk/core/database/cosmosdb/CosmosDBViewMapper.scala](https://codecov.io/gh/apache/openwhisk/pull/4632/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJWaWV3TWFwcGVyLnNjYWxh) | `0% <0%> (-92.6%)` | :arrow_down: | | [...e/database/cosmosdb/cache/ChangeFeedListener.scala](https://codecov.io/gh/apache/openwhisk/pull/4632/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0NoYW5nZUZlZWRMaXN0ZW5lci5zY2FsYQ==) | `0% <0%> (-86.67%)` | :arrow_down: | | [...e/database/cosmosdb/cache/KafkaEventProducer.scala](https://codecov.io/gh/apache/openwhisk/pull/4632/diff?src=pr&el=tree#diff-Y29yZS9jb3Ntb3NkYi9jYWNoZS1pbnZhbGlkYXRvci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL29wZW53aGlzay9jb3JlL2RhdGFiYXNlL2Nvc21vc2RiL2NhY2hlL0thZmthRXZlbnRQcm9kdWNlci5zY2FsYQ==) | `0% <0%> (-76.48%)` | :arrow_down: | | [...whisk/core/database/cosmosdb/CosmosDBSupport.scala](https://codecov.io/gh/apache/openwhisk/pull/4632/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvY29zbW9zZGIvQ29zbW9zREJTdXBwb3J0LnNjYWxh) | `0% <0%> (-74.08%)` | :arrow_down: | | [...a/org/apache/openwhisk/common/ExecutorCloser.scala](https://codecov.io/gh/apache/openwhisk/pull/4632/diff?src=pr&el=tree#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvbW1vbi9FeGVjdXRvckNsb3Nlci5zY2FsYQ==) | `0% <0%> (-66.67%)` | :arrow_down: | | ... and [9 more](https://codecov.io/gh/apache/openwhisk/pull/4632/diff?src=pr&el=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/openwhisk/pull/4632?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/openwhisk/pull/4632?
[GitHub] [openwhisk] jiangpengcheng closed issue #4615: key 'kind' does not exist
jiangpengcheng closed issue #4615: key 'kind' does not exist URL: https://github.com/apache/openwhisk/issues/4615 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] jiangpengcheng commented on issue #4615: key 'kind' does not exist
jiangpengcheng commented on issue #4615: key 'kind' does not exist URL: https://github.com/apache/openwhisk/issues/4615#issuecomment-533430113 closed as merged 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] jiangpengcheng commented on issue #4615: key 'kind' does not exist
jiangpengcheng commented on issue #4615: key 'kind' does not exist URL: https://github.com/apache/openwhisk/issues/4615#issuecomment-533430161 gg 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
