From issues-return-40018-archive-asf-public=cust-asf.ponee.io@openwhisk.apache.org Thu Sep 19 14:24:26 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 0C616180652 for ; Thu, 19 Sep 2019 16:24:25 +0200 (CEST) Received: (qmail 35668 invoked by uid 500); 19 Sep 2019 14:24:25 -0000 Mailing-List: contact issues-help@openwhisk.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@openwhisk.apache.org Delivered-To: mailing list issues@openwhisk.apache.org Received: (qmail 35655 invoked by uid 99); 19 Sep 2019 14:24:25 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 19 Sep 2019 14:24:25 +0000 From: GitBox To: issues@openwhisk.apache.org Subject: [GitHub] [openwhisk] rabbah commented on a change in pull request #4624: Combines active ack and slot release when both are available. Message-ID: <156890306541.25939.2677995520824500483.gitbox@gitbox.apache.org> Date: Thu, 19 Sep 2019 14:24:25 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit 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: users@infra.apache.org With regards, Apache Git Services