bneradt commented on issue #7380: URL: https://github.com/apache/trafficserver/issues/7380#issuecomment-749740797
Considering @masaori335 's observations, I've been looking at the code and reviewing the negative caching patch and I do notice a change that the negative caching fix did that I didn't anticipate. Here was the code that @masaori335 referenced before fix: https://github.com/apache/trafficserver/blob/f3647619921ada5214b78d6ddb594a10fbc99a15/proxy/http/HttpTransact.cc#L4505 Before the patch, the value of `negative_caching` was true iff the response was cacheable because of negative caching configuration. That is, a 200 response would not cause it to be true: only negative status codes that were considered cacheable because negative caching was enabled would have caused negative_caching to be evaluated to true. With the patch, the `cacheable` variable was changed to reflect negative caching configuration to make it clearer. In so doing, I had to recode the evaluation of `negative_caching`. I attempted to keep the value the same while renaming it to `is_cacheable_and_negative_caching_is_enabled` to clarify its intent. In doing so, I missed that the previous nature of `negative_caching` was to be false for positive status codes. Therefore it doesn't behave the same and the branch @masaori335 references now behaves differently, being followed in cases it wouldn't have before. That sounds to me very likely the cause of the problem he's reporting. The fix will be to adjust the evaluation of `is_cacheable_and_negative_caching_is_enabled` to match the previous behavior of `negative_caching`. I'll work with @masaori335 on a patch for this and verify whether this fixes the leak he's seeing. Thank you @masaori335 for the detailed observations. ---------------------------------------------------------------- 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: [email protected]
