style95 commented on code in PR #5348:
URL: https://github.com/apache/openwhisk/pull/5348#discussion_r1012522201


##########
core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/v2/FunctionPullingContainerProxy.scala:
##########
@@ -518,6 +505,8 @@ class FunctionPullingContainerProxy(
         data.action.fullyQualifiedName(withVersion = true),
         data.action.rev,
         Some(data.clientProxy))
+
+    case x: Event if x.event != PingCache => delay

Review Comment:
   This state is a temporary state before removing containers and the proxy 
itself.
   So the next state is always `Removing` as all cases in this state lead to it 
and both cases clean up etcd data properly.
   
   In the `Removing` state, there is no case of moving back to other states.
   It `stays` or `stops`.
   
   ```scala
     when(Removing, unusedTimeout) {
       // only if ClientProxy is closed, ContainerProxy stops. So it is 
important for ClientProxy to send ClientClosed.
       case Event(ClientClosed, _) =>
         stop()
   
       // even if any error occurs, it still waits for ClientClosed event in 
order to be stopped after the client is closed.
       case Event(t: FailureMessage, _) =>
         logging.error(this, s"unable to delete a container due to ${t}")
   
         stay
   
       case Event(StateTimeout, _) =>
         logging.error(this, s"could not receive ClientClosed for 
${unusedTimeout}, so just stop the container proxy.")
   
         stop()
   
       case Event(Remove | GracefulShutdown, _) =>
         stay()
   
       case Event(DetermineKeepContainer(_), _) =>
         stay()
     }
   ```
   
   So I suppose it would be OK to delay.
   



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to