diff --git a/core/backend/options.go b/core/backend/options.go index b56275c55..4f7c81483 100644 --- a/core/backend/options.go +++ b/core/backend/options.go @@ -202,7 +202,15 @@ func ModelOptions(c config.ModelConfig, so *config.ApplicationConfig, opts ...mo model.WithContext(so.Context), model.WithModelID(c.ModelID()), } - if revision, err := config.ModelConfigRevision(&c); err == nil { + // Prefer the revision stamped when the configuration was loaded. c has since + // been merged with this request's prediction parameters (temperature, top_p, + // stop, ...), and hashing it here would produce a different revision for + // every distinct request body — which the controller reads as a config + // change and rejects as stale. Recomputing is the fallback for a config that + // never passed through the loader. + if revision := c.PersistedConfigRevision(); revision != "" { + defOpts = append(defOpts, model.WithConfigRevision(revision)) + } else if revision, err := config.ModelConfigRevision(&c); err == nil { defOpts = append(defOpts, model.WithConfigRevision(revision)) } else { xlog.Warn("Failed to compute model configuration revision", "model", c.ModelID(), "error", err) diff --git a/core/config/model_config.go b/core/config/model_config.go index 1cc7bc903..cacb6f1ec 100644 --- a/core/config/model_config.go +++ b/core/config/model_config.go @@ -43,8 +43,17 @@ type TTSConfig struct { // @Description ModelConfig represents a model configuration type ModelConfig struct { - modelConfigFile string `yaml:"-" json:"-"` - modelTemplate string `yaml:"-" json:"-"` + modelConfigFile string `yaml:"-" json:"-"` + modelTemplate string `yaml:"-" json:"-"` + // persistedConfigRevision is the revision of this model's persisted + // configuration, stamped when the loader materializes it and therefore + // before any per-request override is merged in. The request pipeline + // mutates its copy of a ModelConfig with the caller's sampling parameters + // (temperature, top_p, stop, ...), so hashing the config at load time is + // the only way the controller sees one revision per configuration rather + // than one per request body. Unexported, so it never enters the hash it + // describes and never reaches YAML or JSON. + persistedConfigRevision string `yaml:"-" json:"-"` schema.PredictionOptions `yaml:"parameters,omitempty" json:"parameters,omitempty"` Name string `yaml:"name,omitempty" json:"name,omitempty"` Artifacts []modelartifacts.Spec `yaml:"artifacts,omitempty" json:"artifacts,omitempty"` @@ -1836,6 +1845,27 @@ func (c *ModelConfig) GetModelConfigFile() string { return c.modelConfigFile } +// PersistedConfigRevision returns the revision stamped when this configuration +// was loaded, or "" when it was never stamped (a config synthesized outside the +// loader). Callers that need a revision for a request must prefer this over +// recomputing one from the config they hold: by then the request pipeline has +// merged the caller's prediction parameters into it. +func (c *ModelConfig) PersistedConfigRevision() string { + return c.persistedConfigRevision +} + +// StampPersistedConfigRevision records the revision of this configuration as +// persisted. It is computed from the receiver as-is, so callers must invoke it +// only on a configuration that has not been merged with request overrides. +func (c *ModelConfig) StampPersistedConfigRevision() error { + revision, err := ModelConfigRevision(c) + if err != nil { + return err + } + c.persistedConfigRevision = revision + return nil +} + // GetModelTemplate returns the model's chat template if available func (c *ModelConfig) GetModelTemplate() string { return c.modelTemplate diff --git a/core/config/model_config_loader.go b/core/config/model_config_loader.go index d8bb02e40..4c95a9665 100644 --- a/core/config/model_config_loader.go +++ b/core/config/model_config_loader.go @@ -220,6 +220,15 @@ func (bcl *ModelConfigLoader) LoadModelConfigFileByName(modelName, modelPath str cfg.SetDefaults(append(opts, ModelPath(modelPath))...) + // Stamp the revision here, at the boundary between the persisted + // configuration and the request that is about to override parts of it. + // Everything downstream of this point (the request middleware) merges + // per-request prediction parameters into cfg, so a revision computed later + // would identify the request rather than the configuration. + if err := cfg.StampPersistedConfigRevision(); err != nil { + return nil, fmt.Errorf("stamping config revision for %q: %w", modelName, err) + } + return cfg, nil } diff --git a/core/http/middleware/request_config_revision_test.go b/core/http/middleware/request_config_revision_test.go new file mode 100644 index 000000000..372a033ea --- /dev/null +++ b/core/http/middleware/request_config_revision_test.go @@ -0,0 +1,120 @@ +package middleware_test + +import ( + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + + "github.com/labstack/echo/v4" + "github.com/mudler/LocalAI/core/config" + . "github.com/mudler/LocalAI/core/http/middleware" + "github.com/mudler/LocalAI/core/schema" + "github.com/mudler/LocalAI/pkg/model" + "github.com/mudler/LocalAI/pkg/system" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// The distributed controller pins a model's replicas to the revision of its +// persisted configuration. Inference requests only ever *establish* that +// revision, so a revision that varies per request permanently wedges the model: +// the first request's value is stored, and every later request carrying a +// different one is rejected with "stale model config revision". +var _ = Describe("Model config revision seen by inference requests", func() { + var ( + app *echo.Echo + modelDir string + ) + + // revisionFor drives the real request pipeline (SetModelAndConfig -> + // SetOpenAIRequest) and returns the config revision the handler is left + // holding: the value core/backend.ModelOptions forwards to the model + // router, and that the controller stores as the model's revision. + revisionFor := func(body string) string { + req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + app.ServeHTTP(rec, req) + Expect(rec.Code).To(Equal(http.StatusOK), "request pipeline rejected the request: %s", rec.Body.String()) + // An unstamped config would make every comparison below trivially true. + Expect(rec.Body.String()).ToNot(BeEmpty(), "no config revision reached the handler") + return rec.Body.String() + } + + BeforeEach(func() { + var err error + modelDir, err = os.MkdirTemp("", "localai-revision-models-*") + Expect(err).ToNot(HaveOccurred()) + + Expect(os.WriteFile( + filepath.Join(modelDir, "test-model.yaml"), + []byte("name: test-model\nbackend: llama-cpp\ncontext_size: 4096\n"), + 0o600, + )).To(Succeed()) + + ss := &system.SystemState{Model: system.Model{ModelsPath: modelDir}} + appConfig := config.NewApplicationConfig() + appConfig.SystemState = ss + + mcl := config.NewModelConfigLoader(modelDir) + ml := model.NewModelLoader(ss) + re := NewRequestExtractor(mcl, ml, appConfig) + + app = echo.New() + app.POST("/v1/chat/completions", + func(c echo.Context) error { + if err := re.SetOpenAIRequest(c); err != nil { + return err + } + cfg, ok := c.Get(CONTEXT_LOCALS_KEY_MODEL_CONFIG).(*config.ModelConfig) + Expect(ok).To(BeTrue()) + return c.String(http.StatusOK, cfg.PersistedConfigRevision()) + }, + re.SetModelAndConfig(func() schema.LocalAIRequest { return new(schema.OpenAIRequest) }), + ) + }) + + AfterEach(func() { Expect(os.RemoveAll(modelDir)).To(Succeed()) }) + + It("is identical for requests that differ only in sampling parameters", func() { + baseline := revisionFor(`{"model":"test-model","messages":[{"role":"user","content":"hi"}]}`) + + Expect(revisionFor(`{"model":"test-model","temperature":0.9,"messages":[{"role":"user","content":"hi"}]}`)). + To(Equal(baseline), "temperature must not change the persisted config revision") + Expect(revisionFor(`{"model":"test-model","top_p":0.5,"messages":[{"role":"user","content":"hi"}]}`)). + To(Equal(baseline), "top_p must not change the persisted config revision") + Expect(revisionFor(`{"model":"test-model","top_k":20,"messages":[{"role":"user","content":"hi"}]}`)). + To(Equal(baseline), "top_k must not change the persisted config revision") + Expect(revisionFor(`{"model":"test-model","max_tokens":128,"messages":[{"role":"user","content":"hi"}]}`)). + To(Equal(baseline), "max_tokens must not change the persisted config revision") + Expect(revisionFor(`{"model":"test-model","stop":"STOP","messages":[{"role":"user","content":"hi"}]}`)). + To(Equal(baseline), "stop words must not change the persisted config revision") + }) + + It("is identical for repeated requests carrying the same sampling parameters", func() { + body := `{"model":"test-model","temperature":0.2,"stop":"END","messages":[{"role":"user","content":"hi"}]}` + Expect(revisionFor(body)).To(Equal(revisionFor(body))) + }) + + // The controller compares the revision an inference request establishes + // against the one model administration publishes when a YAML changes. If + // the two paths hash different things, an edited model can never be routed + // again, so they must agree on the same persisted configuration. + It("matches the revision model administration computes for the same config", func() { + ss := &system.SystemState{Model: system.Model{ModelsPath: modelDir}} + appConfig := config.NewApplicationConfig() + appConfig.SystemState = ss + + admin := config.NewModelConfigLoader(modelDir) + Expect(admin.LoadModelConfigsFromPath(modelDir, appConfig.ToConfigLoaderOptions()...)).To(Succeed()) + loaded, ok := admin.GetModelConfig("test-model") + Expect(ok).To(BeTrue()) + adminRevision, err := config.ModelConfigRevision(&loaded) + Expect(err).ToNot(HaveOccurred()) + + Expect(revisionFor(`{"model":"test-model","temperature":0.7,"messages":[{"role":"user","content":"hi"}]}`)). + To(Equal(adminRevision)) + }) +}) diff --git a/docs/content/features/distributed-mode.md b/docs/content/features/distributed-mode.md index a15fef132..3d1e42a85 100644 --- a/docs/content/features/distributed-mode.md +++ b/docs/content/features/distributed-mode.md @@ -1020,6 +1020,12 @@ Notes: - Upgrade the worker when it does not support the exact model-stop request. - Stop and restart the stale backend only as an operational recovery action. LocalAI keeps it non-routable while durable cleanup is pending. +**Requests fail with `stale model config revision` although nobody edited the model:** +- A model's stored revision must describe its persisted configuration. Releases before this fix also hashed the per-request prediction parameters, so the first request after a restart pinned the revision to its own `temperature`, `top_p`, `stop` and similar values. Every later request that sent different values was then rejected. +- Upgrade the frontend replicas first. After the upgrade the revision is stamped when the configuration is loaded, so it no longer depends on the request body. +- The stored revision does not heal on its own, because the recorded value belongs to no persisted configuration. Clear it once per affected model so the next request establishes the correct revision: `DELETE FROM model_config_states WHERE model_name = '';` +- Saving any edit for the model through the API or the WebUI has the same effect, because an edit publishes the current revision. + **Port conflicts on workers:** - Each model gets its own gRPC process on an incrementing port (50051, 50052, ...) - The HTTP file transfer server runs on the base port - 1 (default: 50050)