Skip to content

fix(distributed): exclude pinned models from cluster eviction and idle scale-down - #11872

Open
Dennisadira wants to merge 1 commit into
mudler:masterfrom
Dennisadira:fix/pinned-distributed-eviction-11101
Open

fix(distributed): exclude pinned models from cluster eviction and idle scale-down#11872
Dennisadira wants to merge 1 commit into
mudler:masterfrom
Dennisadira:fix/pinned-distributed-eviction-11101

Conversation

@Dennisadira

Copy link
Copy Markdown
Contributor

Description

Addresses the cluster-side eviction gap in #11101.

pinned: true was only honoured by the per-node WatchDog (idle eviction, LRU limit, memory reclaimer, group eviction). Every distributed eviction path was pinned-blind:

  • SmartRouter.EvictLRUFindLRUModel
  • SmartRouter.evictLRUAndFreeNode (locked SQL selection)
  • ReplicaReconciler.scaleDownIdle

Since eviction is gated on in_flight = 0, a pinned model became eviction-eligible the instant each response completed. Under capacity pressure that surfaces exactly as the report describes: the backend is freed immediately after every request, with the reporter's settings (watchdog off, max_active_backends: 0, reclaimer off) creating no watchdog at all, so the local exclusion never had a chance to apply.

Fix

Wire the model config loader into the router and reconciler through a new PinnedModelResolver seam (mirrors ConcurrencyConflictResolver):

  • EvictLRU passes the pinned set into FindLRUModel's query, so the next-oldest unpinned model is selected instead of the attempt failing
  • evictLRUAndFreeNode filters pinned models inside its locked selection
  • scaleDownIdle skips pinned models entirely

Deliberate teardown (admin unload, model delete, node drain) intentionally still applies to pinned models, as does dead-row reaping.

Tests

  • New core/services/nodes/pinned_eviction_test.go regression specs, verified to fail with the exclusion disabled
  • GetPinnedModelNames specs in the config loader tests
  • Upstream revision_eligibility_test.go updated for the new FindLRUModel signature

Docs

docs/content/advanced/vram-management.md documents that pinned now also protects models in distributed mode.

Notes for reviewers

The pkg/grpc build on master currently needs make protogen-go first (committed pb bindings lag backend.proto); unrelated to this change.

Fixes #11101

…e scale-down

pinned: true was only honoured by the per-node watchdog. Every distributed
eviction path was pinned-blind: the router's LRU eviction (EvictLRU,
evictLRUAndFreeNode) and the replica reconciler's idle scale-down would
happily unload a pinned model — and since eviction is gated on
in_flight = 0, a pinned model became eviction-eligible the instant each
response completed. Under capacity pressure that surfaces as the backend
being freed immediately after every request (mudler#11101).

Wire the model config loader into the router and reconciler through a new
PinnedModelResolver seam (mirroring ConcurrencyConflictResolver):

- EvictLRU passes the pinned set into FindLRUModel's query so the
  next-oldest unpinned model is selected instead of the attempt failing
- evictLRUAndFreeNode filters pinned models inside its locked selection
- scaleDownIdle skips pinned models entirely: trimming to the floor still
  means requests beyond the survivor's capacity pay a cold reload

Deliberate teardown (admin unload, model delete, node drain) intentionally
still applies to pinned models, as does dead-row reaping (state correction,
not eviction).

Regression specs verified to fail with the exclusion disabled.

Addresses the cluster-side eviction gap in mudler#11101

Assisted-by: Claude Code:claude-fable-5 [Claude Code]
Signed-off-by: Adira Denis Muhando <dennisadira@gmail.com>
@localai-org-maint-bot

Copy link
Copy Markdown
Collaborator

This does not compile: the FindLRUModel signature change misses a caller.

core/services/nodes/interfaces.go and registry.go move to FindLRUModel(ctx, nodeID, excludeModels []string), and the in-package callers and fakes are all updated, but tests/e2e/distributed/node_registration_test.go:219 still calls the two-argument form:

vet: tests/e2e/distributed/node_registration_test.go:219:66: not enough arguments in call to registry.FindLRUModel
	have (context.Context, string)
	want (context.Context, string, []string)

Reproduced on the PR head with go vet ./tests/e2e/distributed/.... There is no build tag on that file, so it is a plain build break of the package; it slipped through because the only check that ran on this PR was DCO.

Fix is to pass nil at that call site, matching how reconciler_inflight_leak_test.go and revision_eligibility_test.go were already updated in this PR.

The rest of the change looks good to me: the PinnedModelResolver seam, the reconciler exemption, and the docs update are all consistent, and the comment about deliberate teardown deliberately bypassing the exemption is the right call.

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.

pinned: true model is freed after every request — idle-unload exclusion (#9305) ineffective on v4.7.1

2 participants