feat(cache): add in flight deduping#4459
feat(cache): add in flight deduping#4459MasterPtato wants to merge 1 commit into03-18-fix_cache_clean_up_libfrom
Conversation
|
🚅 Deployed to the rivet-pr-4459 environment in rivet-frontend
|
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
175706a to
17d21f5
Compare
75e1e38 to
bc5d6d3
Compare
|
Code Review: feat(cache): add in flight deduping. Good feature addition. Two correctness issues: (1) Critical - In-flight entries are leaked when the leased getter returns an error. tokio::try_join! propagates via ? before the broadcast send and remove_async cleanup, permanently poisoning the in_flight map for those keys. Fix: restructure so cleanup always runs before the ? propagation. (2) Bug - Broadcast is only sent inside if !entries_values.is_empty(). If getter returns no values, broadcast is never sent and waiters block for the full 5s IN_FLIGHT_TIMEOUT. Fix: send broadcast unconditionally before lease cleanup. Minor: timeout_falls_back_to_getter waits 5s per CI run - make IN_FLIGHT_TIMEOUT configurable for tests. Minor: HashMap deduplication in GetterCtx::new is implicit - add a comment. Nit: broadcast::channel capacity 1 is sufficient since one message per lease. Positive: GetterCtx HashMap refactor is clean (O(1) lookups), CacheKey: Eq+Hash bound is right, &[RawCacheKey] API is better, test split into fetch.rs/in_flight.rs/ttl.rs is much cleaner than the old monolithic integration.rs, and the dedup tests give solid happy-path coverage. |
bc5d6d3 to
b395fae
Compare
17d21f5 to
de18421
Compare
de18421 to
e65f84d
Compare
b395fae to
6570bf2
Compare
6570bf2 to
b46226c
Compare
e65f84d to
65200cf
Compare
65200cf to
ea32d90
Compare
b46226c to
66ec30f
Compare
|
line1\nline2\nline3 |

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: