willi-menapace opened a new pull request, #13916:
URL: https://github.com/apache/trafficserver/pull/13916
`proxy.config.cache.hit_evacuate_percent` is documented as the size of the
region in front of the write cursor, as a percentage of the stripe. When a
fragment in that region is read, it is marked for evacuation.
The window is computed in one unit and compared in another:
- `StripeSM::recompute_hit_evacuate_window()` derives the window from
`data_blocks`. `Stripe::_init_data()` counts `data_blocks` in
`STORE_BLOCK_SIZE` units (8 KiB).
- `StripeSM::within_hit_evacuate_window()` compares the window, and
`data_blocks` itself, against a distance computed from `dir_offset()` and
`write_pos / CACHE_BLOCK_SIZE`. Both of those are in `CACHE_BLOCK_SIZE` units
(512 B).
The ratio is 16, which has two effects:
1. **The window in front of the cursor is 16× smaller than configured.**
`hit_evacuate_percent: 10` covers 0.625 % of the stripe.
2. **The wrap-around branch matches the wrong region.** `-delta >
(data_blocks - hit_evacuate_window) && -delta < data_blocks` is meant to catch
fragments ahead of the cursor after the stripe wraps. With the mixed units it
matches fragments between 5.625 % and 6.25 % of the stripe *behind* the cursor,
which were written only recently. When the cursor is near the end of the
stripe, fragments just past the wrap point are not matched.
The code is the same on master, 10.1.x and 9.2.x (`Vol` in `P_CacheVol.h` /
`Cache.cc`).
## Fix
- Convert `data_blocks` to `CACHE_BLOCK_SIZE` units in both places.
- Make `hit_evacuate_window` an `off_t`. In 512 B units the window no longer
fits in an `int` once a stripe is around 10 TiB at 10 % (a single large span
forms one such stripe). The field is in-memory only; the on-disk format is
unchanged.
## Reproduction
The PR adds a Catch2 case to `src/iocore/cache/unit_tests/test_Stripe.cc`.
It sizes a stripe through `data_blocks`, places the write cursor, and asks
`within_hit_evacuate_window()` about fragments at known distances.
```
cmake -B build -G Ninja -DBUILD_TESTING=ON
cmake --build build --target CacheStripe
./build/src/iocore/cache/CacheStripe "hit evacuation window*"
```
**On master (934c9aa) without the fix:** 10 of 16 assertions fail.
```
CHECK( stripe.hit_evacuate_window == dir_blocks / 10 )
with expansion:
100000 (0x186a0) == 1600000 (0x186a00)
CHECK( in_hit_evacuate_window(stripe, w + percent(90)) ) // 9 % ahead
of the cursor
with expansion:
false
CHECK_FALSE( in_hit_evacuate_window(stripe, w - percent(59)) ) // 5.9 %
behind the cursor
with expansion:
!true
...
test cases: 1 | 1 failed
assertions: 16 | 6 passed | 10 failed
```
**With the fix:** `All tests passed (16 assertions in 1 test case)`. The
whole `CacheStripe` binary also passes (117 assertions in 5 test cases).
## Behavior change
Deployments that set `hit_evacuate_percent` today evacuate from a region
about 16× smaller than they configured. After this change they get the
documented region, so they will evacuate more fragments, and copy and write
more data, at the same setting. This probably deserves a line in the release
notes. `proxy.config.cache.hit_evacuate_size_limit` still caps the size of the
objects considered.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
