bneradt commented on code in PR #13608:
URL: https://github.com/apache/trafficserver/pull/13608#discussion_r4021985938
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -683,21 +671,17 @@ Directory::remove(const CacheKey *key, StripeSM *stripe,
Dir *del)
int b = key->slice32(1) % this->buckets;
Dir *seg = this->get_segment(s);
Dir *e = nullptr, *p = nullptr;
-#ifdef LOOP_CHECK_MODE
- int loop_count = 0;
-#endif
CHECK_DIR(vol);
e = dir_bucket(b, seg);
if (dir_offset(e)) {
+ int loop_count = 0;
do {
-#ifdef LOOP_CHECK_MODE
- loop_count++;
- if (loop_count > DIR_LOOP_THRESHOLD) {
- if (this->bucket_loop_fix(dir_bucket(b, seg), s))
- return 0;
+ // Past the longest legitimate chain, so this is provably a cycle.
+ if (++loop_count > this->max_bucket_depth()) {
+ this->bucket_loop_fix(dir_bucket(b, seg), s);
Review Comment:
[P2] Account for discarded entries when repairing a segment
Confirming the gauge issue mentioned in the automated review summary: this
newly enabled production path calls bucket_loop_fix(), which wipes the entire
segment through init_segment(), but neither helper decrements
cache_rsb.direntries_used or stripe->cache_vol->vol_rsb.direntries_used. For
example, repairing a segment containing five counted entries leaves both gauges
five too high; a subsequent insert increments them again even though only one
entry remains. Ordinary removals cannot correct the error because the discarded
entries no longer exist. Please account for the occupied entries discarded by
runtime repairs in both gauges, and extend the repair tests to assert the
before/after counts. Count physical entries rather than walking the corrupt
chain.
--
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]
