Copilot commented on code in PR #13840:
URL: https://github.com/apache/trafficserver/pull/13840#discussion_r4279497940
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -497,19 +495,13 @@ Directory::probe(const CacheKey *key, StripeSM *stripe,
Dir *result, Dir **last_
*last_collision = e;
ink_assert(dir_offset(e) * CACHE_BLOCK_SIZE < stripe->len);
return 1;
- } else { // delete the invalid entry
- ts::Metrics::Gauge::decrement(cache_rsb.direntries_used);
-
ts::Metrics::Gauge::decrement(stripe->cache_vol->vol_rsb.direntries_used);
- ATS_PROBE7(cache_dir_remove_invalid, stripe->fd, s, dir_to_offset(e,
seg), dir_offset(e), dir_approx_size(e),
- key->slice64(0), key->slice64(1));
- e = this->delete_entry(e, p, s);
- continue;
}
+ // Stale entry, left in place: reclaiming it here would make every
lookup a writer. A writer collects it
+ // instead -- Directory::cleanup() at each cache cycle wrap, or
freelist_clean() on freelist exhaustion.
Review Comment:
Retaining stale entries makes writer matching ambiguous when an offset is
reused. If a stale entry precedes a live entry with the same tag and offset but
opposite phases, `probe()` now skips the stale entry and returns the live one.
However, `remove()` and `overwrite()` match only tag and offset
(CacheDir.cc:703 and :627), so they select the stale entry instead.
`CacheVC::removeEvent()` then reports successful removal while the live object
remains cached; overwrite can likewise update the wrong entry. Make writer
matching or writer-side cleanup distinguish these entries, and add
removal/overwrite regressions with stale and live entries sharing an offset. If
matching is made phase-aware, also update callers that construct synthetic
targets, such as the `del_dir` in StripeSM.cc:757–762.
--
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]
