brbzull0 commented on code in PR #13768:
URL: https://github.com/apache/trafficserver/pull/13768#discussion_r4163756117


##########
src/iocore/cache/CacheVC.cc:
##########
@@ -403,6 +403,15 @@ CacheVC::handleReadDone(int event, Event * /* e ATS_UNUSED 
*/)
       goto Ldone;
     }
 
+    // Everything below trusts len and hlen, and STORE_COLLISION lets a doc 
that is not ours get this far.
+    if (doc->magic == DOC_MAGIC &&

Review Comment:
   question (non-blocking): This check runs before the key comparison, so it 
also marks docs whose key doesn't match. Head and Earliest handle that well: 
they drop the bad dir and probe again. The Middle read path 
(`CacheRead.cc:485-493`) doesn't: on `DOC_CORRUPT` it goes to `Lerror`. Before 
this change, a key-mismatched doc there fell through to `directory.probe()` and 
could still find the right fragment further down the collision chain. Now that 
same inconsistent dir fails the whole read. It should be rare, since it needs a 
tag collision plus a size mismatch. Is failing the read intended there, or 
should the mismatched-key case keep probing?



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