Skip to content

fix: refresh cache loading overhead bounds - #108

Open
sparknack wants to merge 2 commits into
zilliztech:masterfrom
sparknack:fix/refresh-loading-overhead
Open

fix: refresh cache loading overhead bounds#108
sparknack wants to merge 2 commits into
zilliztech:masterfrom
sparknack:fix/refresh-loading-overhead

Conversation

@sparknack

@sparknack sparknack commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • re-read translator loading-overhead config before every CacheSlot load
  • track each live registration cap and use the maximum as the shared group bound
  • support safe bound increases and decreases with exact failed-reservation rollback
  • refresh tracker state without serializing on the DList-wide lock

Contract

  • configured dimensions and group names stay stable for the CacheSlot lifetime
  • use INT64_MAX when a dimension starts uncapped but may become capped later

Test Plan

  • cmake --build . -j8
  • ./build/test/test_cachinglayer/cachinglayer_test (143 passed)
  • ./build/test/all_tests (112 passed)

Refs: milvus-io/milvus#51403

Refresh CacheSlot loading-overhead configuration before each load. Track each registration's cap contribution so shared groups can safely recompute their effective bound, and roll back failed reservations with the exact tracker delta.

Signed-off-by: Shawn Wang <shawn.wang@zilliz.com>
Let loading-overhead refresh rely on the tracker mutex instead of the DList-wide lock. Add a regression test proving refresh can complete while the list lock is held.

Signed-off-by: Shawn Wang <shawn.wang@zilliz.com>
std::vector<cid_t> loading_cids;
try {
if (auto loading_overhead = translator_->loading_overhead_config()) {
dlist_->RefreshLoadingOverheadUpperBound(overhead_handle_, *loading_overhead);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RunLoad refreshes only the loading slot's own overhead_handle_, but a group's effective upper bound is the max over every live registration's contribution (upper_bound = upper_bound_ref_counts.rbegin()->first). A slot that finished loading and stays resident never re-runs RunLoad, so its contribution is never refreshed; if the shared config decreases, that stale higher contribution pins the group bound. For the documented pattern of registering INT64_MAX at construction for a dimension that may later become capped (Translator.h), a single resident slot keeps the group max at INT64_MAX, so the group cap stays effectively unenforced (loading overhead is then bounded only by the global DList limit, not the configured group cap) until that slot reloads or unregisters. Refresh all live registrations on a config change, or track the group's target bound centrally, rather than only the caller's handle.

// contribution is updated. The group's effective bound remains the maximum
// contribution from all live registrations.
void
RefreshUpperBound(uint64_t handle, const LoadingOverheadConfig& config) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RefreshUpperBound updates a group's upper bound but never re-evaluates the DList waiting queue. When a refresh lowers a group's cap, a blocked reservation for that same group needs a smaller overhead delta (reserveDimension caps delta at min(sum, upper_bound) - overhead_reserved) and may become admissible without any resource being freed, yet nothing schedules handleWaitingRequests. With background eviction disabled (the EvictionConfig default) and no other Release/touch activity, such a waiter can stall until its timeout even though it is now satisfiable. Schedule a handleWaitingRequests pass when a refresh yields an effective bound decrease; it must be scheduled asynchronously (e.g. on the event-base thread) to preserve this PR's goal of refreshing without taking the DList list lock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants