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_r324425930
 
 

 ##########
 File path: 
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala
 ##########
 @@ -64,43 +64,34 @@ case class ActivationMessage(override val transid: 
TransactionId,
   def causedBySequence: Boolean = cause.isDefined
 }
 
-object ActivationMessage extends DefaultJsonProtocol {
-
-  def parse(msg: String) = Try(serdes.read(msg.parseJson))
-
-  private implicit val fqnSerdes = FullyQualifiedEntityName.serdes
-  implicit val serdes = jsonFormat11(ActivationMessage.apply)
-}
-
 /**
  * Message that is sent from the invoker to the controller after action is 
completed or after slot is free again for
  * new actions.
  */
 abstract class AcknowledegmentMessage(private val tid: TransactionId) extends 
Message {
   override val transid: TransactionId = tid
-  override def serialize: String = {
-    AcknowledegmentMessage.serdes.write(this).compactPrint
-  }
+  override def serialize: String = 
AcknowledegmentMessage.serdes.write(this).compactPrint
+  def toJson: JsValue
 }
 
 /**
- * This message is sent from the invoker to the controller, after the slot of 
an invoker that has been used by the
- * current action, is free again (after log collection)
+ * This message is sent from an invoker to the controller, after the resource 
slot in the invoke, used by the
+ * corresponding activation, is free again (i.e., after log collection). In 
some cases, the activation result is
+ * ready and the slot is freed at the same time. In such cases, the completion 
message carries the result as well.
+ * This is reflected by the of a Right() `response` and the param `result` is 
set to true.
+ * In some cases the `result` is true but the response is Left() if the 
message was too large for the message bus.
  */
 case class CompletionMessage(override val transid: TransactionId,
-                             activationId: ActivationId,
-                             isSystemError: Boolean,
+                             response: Either[ActivationId, WhiskActivation],
+                             result: Boolean, // true iff the message is a 
combined active ack and slot released
 
 Review comment:
   When `result` is false:
   1. `Left`: this is the current situation and can occur during split phase 
notification.
   1. `Right`: cannot happen, would be a bug (this should be an active ack 
instead).
   
   When `result` is true:
   1. `Right`: this combines the active ack and slot release and occurs during 
system generate activations for failure scenarios.
   2. `Left`: like the previous case but occurs when the message is too large 
to cross the event bus and the sender converts the Right to a Left.
   
   Since there are 3 possible scenarios, `Either` by itself is not enough, and 
so I chose to encode the extra bit with `result` as a boolean.
   
   An alternative would be `Either[ActivationId, Option[Either[ActivationId, 
WhiskActivation]]` where `Left` represents the split phase and `Right(Some)` 
represents the other two cases.

----------------------------------------------------------------
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

Reply via email to