You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by GitBox <gi...@apache.org> on 2022/01/19 01:51:37 UTC

[GitHub] [trafficserver] bneradt opened a new issue #8616: cache-request-method AuTest fails on master

bneradt opened a new issue #8616:
URL: https://github.com/apache/trafficserver/issues/8616


   After merging in this PR:
   https://github.com/apache/trafficserver/pull/8545
   
   CI now fails.
   
   This is the sequence of events that led to this:
   
   1. PR #8545 was created. CI ran and passed.
   2. While that PR was up for review, PR #8484 was put up and merged in.
   3. PR #8545 was then merged in.
   4. CI now fails for the test added in #8545 to verify its behavior.
   
   It looks like #8484 breaks the patch in #8545 by causing the `TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE` to fire earlier, before the revalidation check is performed.
   
   Here are the logs for a relevant transaction:
   
   ```
   1406 [Jan 19 01:46:17.613] [ET_NET 3] DEBUG: <InkAPI.cc:5422 (TSHttpTxnCacheLookupStatusGet)> (cache) Entering TSHttpTxnCacheLookupStatusGet: 5                                                                      
   1407 [Jan 19 01:46:17.613] [ET_NET 3] DEBUG: <HttpSM.cc:1429 (state_api_callback)> (http) [4] [&HttpSM::state_api_callback, HTTP_API_CONTINUE/TS_EVENT_HTTP_CONTINUE]                                            
   1408 [Jan 19 01:46:17.613] [ET_NET 3] DEBUG: <HttpSM.cc:1469 (state_api_callout)> (http) [4] [&HttpSM::state_api_callout, HTTP_API_CONTINUE/TS_EVENT_HTTP_CONTINUE]                          
   1409 [Jan 19 01:46:17.613] [ET_NET 3] DEBUG: <HttpTransact.cc:2823 (HandleCacheOpenReadHit)> (http_seq) [4] [HttpTransact::HandleCacheOpenReadHit] Authentication not needed                              
   1410 [Jan 19 01:46:17.613] [ET_NET 3] DEBUG: <HttpTransact.cc:2885 (HandleCacheOpenReadHit)> (http) [4] setting cache_hit_but_revalidate                                                                                 
   1411 [Jan 19 01:46:17.613] [ET_NET 3] DEBUG: <HttpTransact.cc:2894 (HandleCacheOpenReadHit)> (http_trans) [4] CacheOpenRead --- needs_auth          = 0                                           
   1412 [Jan 19 01:46:17.613] [ET_NET 3] DEBUG: <HttpTransact.cc:2895 (HandleCacheOpenReadHit)> (http_trans) [4] CacheOpenRead --- needs_revalidate    = 0                                                                            
   1413 [Jan 19 01:46:17.613] [ET_NET 3] DEBUG: <HttpTransact.cc:2896 (HandleCacheOpenReadHit)> (http_trans) [4] CacheOpenRead --- response_returnable = 0                                      
   1414 [Jan 19 01:46:17.613] [ET_NET 3] DEBUG: <HttpTransact.cc:2897 (HandleCacheOpenReadHit)> (http_trans) [4] CacheOpenRead --- needs_cache_auth    = 0                                                
   1415 [Jan 19 01:46:17.613] [ET_NET 3] DEBUG: <HttpTransact.cc:2898 (HandleCacheOpenReadHit)> (http_trans) [4] CacheOpenRead --- send_revalidate     = 1                                                                                                       
   1416 [Jan 19 01:46:17.613] [ET_NET 3] DEBUG: <HttpTransact.cc:2901 (HandleCacheOpenReadHit)> (http_trans) [4] CacheOpenRead --- HIT-STALE                                                                   
   1417 [Jan 19 01:46:17.613] [ET_NET 3] DEBUG: <HttpTransact.cc:2903 (HandleCacheOpenReadHit)> (http_seq) [4] [HttpTransact::HandleCacheOpenReadHit] Revalidate document with server                                 
   ```
   
   Notice that the behavior of ATS is still correct: the send_revalidate flag is true. But the `TSHttpTxnCacheLookupStatusGet` is called before this state is detected, therefore the plugin gets the wrong status flag (fresh instead of a miss).
   
   It looks like with #8484, the `need_to_revalidate` revalidate function is no longer called before `TSHttpTxnCacheLookupStatusGet` is called, and thus the `cache_hit_but_revalidate` flag isn't set yet.


-- 
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: issues-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] bneradt commented on issue #8616: cache-request-method AuTest fails on master

Posted by GitBox <gi...@apache.org>.
bneradt commented on issue #8616:
URL: https://github.com/apache/trafficserver/issues/8616#issuecomment-1016015955


   Oh, I see. With #8484, need_to_revalidate is no longer ever called:
   https://github.com/apache/trafficserver/blob/c0e3ddb985689ae2f71ca486421caf67cc83cfc3/proxy/http/HttpTransact.cc#L2702
   
   @serrislew : I think we maybe want to call need_to_revalidate here?
   https://github.com/apache/trafficserver/pull/8484/files#diff-7f286e5169bc2d28c13f1ceaf3bb482b36cdb0743fbac4040fd4cd5a9c0897c1R1973
   
   (That doesn't fix this issue, but it's an observation I'm making while looking at the code.)


-- 
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: issues-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] bneradt closed issue #8616: cache-request-method AuTest fails on master

Posted by GitBox <gi...@apache.org>.
bneradt closed issue #8616:
URL: https://github.com/apache/trafficserver/issues/8616


   


-- 
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: issues-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org