[GitHub] [openwhisk] bdoyle0182 commented on issue #5054: Various Potential Bugs in Container Pool and Container Proxy

[GitBox <gi...@apache.org>]

bdoyle0182 commented on issue #5054:
URL: https://github.com/apache/openwhisk/issues/5054#issuecomment-767957171


   1. I agree that was my conclusion on what was meant to be included there.
   2. Are you saying we should add back the `processBufferOrFeed` there? If so I think I agree.
   4. The issue in my opinion is that the cases that are handled by the Removing state have changed so it's not a catch all for all future failures in the commit I linked for this one. If that `container.destroy()` fails, it will pipe the failure to `Removing` but only a failure when the container is `WarmedData` is caught.
   
   I'm also a little concerned that we give up so easily on attempting to destroy the container. If that `container.destroy()` call fails, we cease tracking of the container immediately. I'm not sure if there's a more graceful or more guaranteed way of handling this other than coding in retries.
   
   5. I agree this isn't a major concern. My only concern was it pulling extra things into memory too early, but not too big of a deal.
   
   Thanks for taking the time to sanity check all of these for me. At the very least we should fix 1. asap. If I find any other logic that I question in the container management logic I'll be sure to add again to get some other opinions. 
   
   You're no longer using the container proxy / pool, is that because of the new scheduler? Will that be a part of the scheduler change?
   


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