masaori335 commented on code in PR #13840:
URL: https://github.com/apache/trafficserver/pull/13840#discussion_r4279922877
##########
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:
Valid — the old probe() deletion is what kept this from happening, since
removeEvent() probes first. Fixed in 19faf693db: remove() and overwrite() now
match tag, offset and phase, which records the cycle an entry was written in. A
stale and a live entry at the same offset always differ in phase (an entry two
cycles old is swept by the wrap cleanup() first). The synthetic del_dir in the
write-error path now carries the current phase. Added regressions for both
writers with a stale and a live entry sharing tag and offset.
--
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]
