[Core] Prioritize real downloads over validation replay#662
Conversation
c1539b0 to
6d93431
Compare
| } | ||
|
|
||
| func (s *Gopher) Run(stopCh <-chan struct{}, numWorker int, numHighPriorityWorker int) { | ||
| s.captureStartupReadyModels(context.Background()) |
There was a problem hiding this comment.
captureStartupReadyModels(context.Background()) does a live ConfigMaps().Get() as the first statement of Run() — the reconciler, dispatcher, and every worker wait on it. The failure path already degrades gracefully to an empty snapshot (today's behavior), so a context.WithTimeout is free insurance against a throttled API server stalling startup. Relatedly, on a cold node the ConfigMap doesn't exist yet, so getConfigMap logs at Errorf and this logs Warnf — an alarming pair for a normal first boot. Consider special-casing apierrors.IsNotFound.
There was a problem hiding this comment.
Addressed. Startup snapshot now uses a fixed 5s startup-only timeout, and NotFound is treated as a cold start with info level logging instead of warning/error noise.
| if task.TaskType == Delete { | ||
| s.cancelActiveDownload(task) | ||
| } | ||
| if s.deferStartupReadyValidationIfLocalPathExists(task) { |
There was a problem hiding this comment.
This classifier is redundant.
For task sits in a normal lane. the funtion markValidationOnlyIfStartupReady() in #201 already classified it on that enqueue. The deferStartupReadyValidationIfLocalPathExists call the same function markValidationOnlyIfStartupReady() and it will return false because task.NormalValidationOnly is true.
For task demote from high lane. markValidationOnlyIfStartupReady in demoteToNormalPriority and markValidationOnlyIfStartupReady() in #201 already classified it during demote's re-enqueue.
There was a problem hiding this comment.
Good catch! Startup revalidation is now classified before enqueue, and the queue itself defers RevalidationReplay behind normal downloads, so the worker-side requeue wrapper is no longer needed.
Addressed
4 participants
What this PR does
Splits model-agent normal work into separate download and validation-only FIFO lanes, then places already-Ready OCI models with existing local artifacts behind real download work.
It also snapshots models that were already Ready on this node before reconciliation marks restart replay tasks as Updating, and adds tests for queue ordering, delete superseding, startup classification, and demotion behavior.
Why we need it
After model-agent restart, many already-downloaded models can be replayed as download tasks just to revalidate local artifacts. Those validation-heavy tasks can block real downloads for new models.
This change keeps real downloads ahead of validation replay without moving real download work onto the high-priority worker.
Fixes #
How to test
go test -count=1 ./pkg/modelagentChecklist
make testpasses locally