[GitHub] [trafficserver] shinrich opened a new pull request #7819: Remove a layer of Action from HttpCacheSM

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

shinrich opened a new pull request #7819:
URL: https://github.com/apache/trafficserver/pull/7819


   Yet another attempt to address issue #7705.  Based on the discussion on PR #7807, @masaori335 convinced me that the captive_action in HttpCacheSM is not doing anything for us.  We would be better off passing through the pending_action that we received from the CacheProcessor (which points at the _action member of the CacheVC).  The only point of the pending_action member in HttpSM is to cancel outstanding actions if the HttpSM goes away.  Nothing looks at the cancel bit of the HttpCacheSM other than some asserts.
   
   @SolidWallOfCode and I reviewed the lifecycle of the HttpCacheSM::pending_action because we were concerned that the current logic would lose the CacheVC actions in case the HttpSM goes away while the cache is still opening.  However, ultimately the HttpSM will close the CacheVC's which will clean up outstanding actions in HttpSM::kill_this
   
   ```
       cache_sm.end_both();
       transform_cache_sm.end_both();
   ```
   
   So we can safely return the HttpCacheSM::pending_action or ACTION_RESULT_DONE instead of the captive action.  It doesn't matter that the HttpSM cancels the action through its pending_action member since the CacheVC cleanup will ultimately do it.  However, it is important that an action is returned so HttpSM knows whether the open action completed immediately or not.
   
   This should solve the assert by removing the assert and associated data structure.
   
   In fact, I think we can get rid of the HttpCacheSM::pending_action member too.  I'll add another commit to do that.
   
   This closes #7705


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

[GitHub] [trafficserver] shinrich commented on pull request #7819: Remove a layer of Action from HttpCacheSM

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

shinrich commented on pull request #7819:
URL: https://github.com/apache/trafficserver/pull/7819#issuecomment-839853457


   Nevermind.   Cannot get rid of HttpCacheSM::pending_action because of do_schedule_in.  I think we are leaking an action there in any case.  I will put up a separate PR to address that.


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

[GitHub] [trafficserver] shinrich closed pull request #7819: Remove a layer of Action from HttpCacheSM

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

shinrich closed pull request #7819:
URL: https://github.com/apache/trafficserver/pull/7819


   


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

[GitHub] [trafficserver] shinrich commented on pull request #7819: Remove a layer of Action from HttpCacheSM

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

shinrich commented on pull request #7819:
URL: https://github.com/apache/trafficserver/pull/7819#issuecomment-839860155


   Closing this.  Looking more at the HttpCacheSM::do_schedule_in() I think the captive_action is necessary to solve canceling that action.  The captive_action::cancel() passes through to cancel the pending_action member of the HttpCacheSM.  As long as the HttpSM.cc is tracking the captive_action via its pending_action, all should be well.
   
   Now back to figuring out the meaning of those asserts and cancel bit fiddling.


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