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: [email protected]

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


Reply via email to