Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions core/application/startup.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ func New(opts ...config.AppOption) (*Application, error) {
}

// Initialize distributed mode services (NATS, object storage, node registry)
// revisionStore is built inside the distributed block below but used after
// the model configs are loaded, so it is declared out here.
var revisionStore modeladmin.RevisionStore

distSvc, err := initDistributed(options, application.authDB, application.ModelConfigLoader())
if err != nil {
return nil, fmt.Errorf("distributed mode initialization failed: %w", err)
Expand Down Expand Up @@ -373,16 +377,9 @@ func New(opts ...config.AppOption) (*Application, error) {
cfgLoaderOpts := options.ToConfigLoaderOptions()
modelRevisionLifecycle := modeladmin.NewDistributedModelRevisionLifecycle(distSvc.Registry, distSvc.ModelCleanup)
gs.SetModelRevisionLifecycle(modelRevisionLifecycle)
// Bring the controller's stored revisions back in line with the
// configuration on disk. An inference request may only establish a
// revision, never replace one, so a model whose stored value had
// drifted stayed unroutable until someone deleted the row.
if err := modeladmin.ResyncModelConfigRevisions(options.Context,
application.ModelConfigLoader(),
modeladmin.NewRevisionStore(distSvc.Registry, modelRevisionLifecycle),
); err != nil {
xlog.Warn("Failed to resync model config revisions", "error", err)
}
// Captured here, used after the model configs are loaded below: the
// resync reads the loader, which is still empty at this point.
revisionStore = modeladmin.NewRevisionStore(distSvc.Registry, modelRevisionLifecycle)
gs.OnModelsChanged = func(evt messaging.CacheInvalidateEvent) {
// ApplyRemoteChange honors the op: a "delete" prunes the element
// (a reload-from-path is additive and cannot drop it), anything
Expand Down Expand Up @@ -429,6 +426,18 @@ func New(opts ...config.AppOption) (*Application, error) {
xlog.Error("error loading config files", "error", err)
}

// Bring the controller's stored revisions back in line with the
// configuration just loaded. An inference request may only establish a
// revision, never replace one, so a model whose stored value has drifted
// stays unroutable until something republishes it. This has to run after
// the load above: the loader is empty until then, and a resync against an
// empty loader silently reconciles nothing.
if revisionStore != nil {
if err := modeladmin.ResyncModelConfigRevisions(options.Context, application.ModelConfigLoader(), revisionStore); err != nil {
xlog.Warn("Failed to resync model config revisions", "error", err)
}
}

if err := gallery.RegisterBackends(options.SystemState, application.ModelLoader()); err != nil {
xlog.Error("error registering external backends", "error", err)
}
Expand Down
11 changes: 10 additions & 1 deletion core/services/modeladmin/revision_resync.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,17 @@ func ResyncModelConfigRevisions(ctx context.Context, loader *config.ModelConfigL
return nil
}

configs := loader.GetAllModelsConfigs()
if len(configs) == 0 {
// Reconciling nothing is indistinguishable from reconciling correctly,
// which is how a caller that ran this before the configs were loaded
// went unnoticed. Say so rather than report success.
xlog.Warn("Skipping model config revision resync: no model configurations are loaded")
return nil
}

var transitions []ModelRevisionTransition
for _, cfg := range loader.GetAllModelsConfigs() {
for _, cfg := range configs {
want, err := config.ModelConfigRevision(&cfg)
if err != nil {
return fmt.Errorf("compute config revision for %q: %w", cfg.Name, err)
Expand Down
15 changes: 15 additions & 0 deletions core/services/modeladmin/revision_resync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,18 @@ var _ = Describe("ResyncModelConfigRevisions", func() {
Expect(store.applied).To(BeEmpty())
})
})

// Running the resync before the model configs are loaded reconciled nothing
// while reporting success, which is how a mis-ordered startup call went
// unnoticed. An empty loader is now called out instead of looking like a
// clean run.
var _ = Describe("ResyncModelConfigRevisions with nothing loaded", func() {
It("does not touch stored revisions when no configs are loaded", func() {
dir := GinkgoT().TempDir()
loader := config.NewModelConfigLoader(dir)
store := &stubRevisionStore{stored: map[string]string{"served-before": "stale"}}

Expect(ResyncModelConfigRevisions(context.Background(), loader, store)).To(Succeed())
Expect(store.applied).To(BeEmpty())
})
})
Loading