mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(coderd): invalidate chatd provider cache on AI provider changes (#26987)
chatd subscribes to `ChatConfigEventChannel` and invalidates its provider cache on a `providers` event kind, but nothing ever published that kind. AI provider CRUD only publishes on `AIProvidersChangedChannel` (consumed by aibridged and aibridgeproxyd), so chatd's provider cache only converged via its 10 second TTL. Subscribe chatd to the same `AIProvidersChangedChannel` publish instead of adding a second publish, per the review feedback on #26207: one publish, multiple subscribers. The now-unused `ChatConfigEventProviders` kind is removed so `ChatConfigEvent` stays scoped to model configs, user prompts, and advisor config, and can't regrow a dead subscriber. Follow-up to CRF-5 from the review of #25673. Supersedes #26207. Closes CODAGT-499
This commit is contained in:
@@ -492,8 +492,8 @@ func (api *API) aiProvidersDelete(rw http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
|
||||
// publishAIProvidersChanged notifies subscribers (aibridged,
|
||||
// aibridgeproxyd) that the live provider set changed and they should
|
||||
// refetch from the database. Pubsub failures are logged but not
|
||||
// aibridgeproxyd, chatd) that the live provider set changed and they
|
||||
// should refetch from the database. Pubsub failures are logged but not
|
||||
// propagated: subscribers refresh authoritatively from the DB, so a
|
||||
// dropped notification only delays convergence.
|
||||
func (api *API) publishAIProvidersChanged(ctx context.Context) {
|
||||
|
||||
@@ -2,8 +2,8 @@ package pubsub
|
||||
|
||||
// AIProvidersChangedChannel is the pubsub channel that carries AI
|
||||
// provider lifecycle events: provider create / update / soft-delete
|
||||
// and key insert / delete. Subscribers (aibridged, aibridgeproxyd)
|
||||
// reload their in-memory provider snapshot on receipt.
|
||||
// and key insert / delete. Subscribers (aibridged, aibridgeproxyd,
|
||||
// chatd) reload their in-memory provider snapshot on receipt.
|
||||
//
|
||||
// The payload is an empty invalidation hint; subscribers refetch the
|
||||
// authoritative state from the database, so dropped messages only
|
||||
|
||||
@@ -9,7 +9,7 @@ import (
|
||||
)
|
||||
|
||||
// ChatConfigEventChannel is the pubsub channel for chat config
|
||||
// changes (providers, model configs, user prompts, advisor config).
|
||||
// changes (model configs, user prompts, advisor config).
|
||||
// All replicas subscribe to this channel to invalidate their local
|
||||
// caches.
|
||||
const ChatConfigEventChannel = "chat:config_change"
|
||||
@@ -33,13 +33,11 @@ func HandleChatConfigEvent(cb func(ctx context.Context, payload ChatConfigEvent,
|
||||
}
|
||||
|
||||
// ChatConfigEvent is published when chat configuration changes
|
||||
// (provider CRUD, model config CRUD, user prompt updates, or advisor
|
||||
// config updates). Subscribers use this to invalidate their local
|
||||
// caches.
|
||||
// (model config CRUD, user prompt updates, or advisor config
|
||||
// updates). Subscribers use this to invalidate their local caches.
|
||||
type ChatConfigEvent struct {
|
||||
Kind ChatConfigEventKind `json:"kind"`
|
||||
// EntityID carries context for the invalidation:
|
||||
// - For providers: uuid.Nil (all providers are invalidated).
|
||||
// - For model configs: the specific config ID.
|
||||
// - For user prompts: the user ID.
|
||||
// - For advisor config: uuid.Nil (singleton site-config row).
|
||||
@@ -49,7 +47,6 @@ type ChatConfigEvent struct {
|
||||
type ChatConfigEventKind string
|
||||
|
||||
const (
|
||||
ChatConfigEventProviders ChatConfigEventKind = "providers"
|
||||
ChatConfigEventModelConfig ChatConfigEventKind = "model_config"
|
||||
ChatConfigEventUserPrompt ChatConfigEventKind = "user_prompt"
|
||||
ChatConfigEventAdvisorConfig ChatConfigEventKind = "advisor_config"
|
||||
|
||||
+21
-2
@@ -179,6 +179,7 @@ type Server struct {
|
||||
debugSvcInit sync.Once
|
||||
configCache *chatConfigCache
|
||||
configCacheUnsubscribe func()
|
||||
providerCacheUnsubscribe func()
|
||||
|
||||
usageTracker *workspacestats.UsageTracker
|
||||
clock quartz.Clock
|
||||
@@ -3300,8 +3301,6 @@ func New(ps pubsub.Pubsub, cfg Config) *Server {
|
||||
return
|
||||
}
|
||||
switch ev.Kind {
|
||||
case coderdpubsub.ChatConfigEventProviders:
|
||||
p.configCache.InvalidateProviders()
|
||||
case coderdpubsub.ChatConfigEventModelConfig:
|
||||
p.configCache.InvalidateModelConfig(ev.EntityID)
|
||||
case coderdpubsub.ChatConfigEventUserPrompt:
|
||||
@@ -3317,6 +3316,22 @@ func New(ps pubsub.Pubsub, cfg Config) *Server {
|
||||
p.configCacheUnsubscribe = cancelConfigSub
|
||||
}
|
||||
|
||||
cancelProviderSub, err := p.pubsub.SubscribeWithErr(
|
||||
coderdpubsub.AIProvidersChangedChannel,
|
||||
func(cbCtx context.Context, _ []byte, err error) {
|
||||
if err != nil {
|
||||
p.logger.Warn(cbCtx, "ai providers changed event error", slog.Error(err))
|
||||
return
|
||||
}
|
||||
p.configCache.InvalidateProviders()
|
||||
},
|
||||
)
|
||||
if err != nil {
|
||||
p.logger.Error(ctx, "subscribe to ai providers changed events", slog.Error(err))
|
||||
} else {
|
||||
p.providerCacheUnsubscribe = cancelProviderSub
|
||||
}
|
||||
|
||||
p.ctx = ctx
|
||||
|
||||
// Spawn background goroutines that all servers need.
|
||||
@@ -4903,6 +4918,10 @@ func (p *Server) Close() error {
|
||||
p.configCacheUnsubscribe = nil
|
||||
unsub()
|
||||
}
|
||||
if unsub := p.providerCacheUnsubscribe; unsub != nil {
|
||||
p.providerCacheUnsubscribe = nil
|
||||
unsub()
|
||||
}
|
||||
if p.chatWorker != nil {
|
||||
if err := p.chatWorker.Close(); err != nil {
|
||||
p.logger.Warn(context.Background(), "failed to close chat worker", slog.Error(err))
|
||||
|
||||
@@ -14,6 +14,9 @@ import (
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/database/dbtestutil"
|
||||
coderdpubsub "github.com/coder/coder/v2/coderd/pubsub"
|
||||
"github.com/coder/coder/v2/coderd/x/chatd/chatprovider"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
"github.com/coder/quartz"
|
||||
@@ -1198,3 +1201,19 @@ func TestConfigCache_InvalidateAdvisorConfig_BlocksStaleInFlight(t *testing.T) {
|
||||
require.EqualValues(t, 2, third.MaxUsesPerRun)
|
||||
require.Equal(t, int32(2), store.advisorConfigCalls.Load())
|
||||
}
|
||||
|
||||
func TestConfigCache_InvalidatesProvidersOnAIProvidersChangedEvent(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
|
||||
|
||||
// The generation counter only advances through InvalidateProviders,
|
||||
// so this cannot false-pass via TTL expiry.
|
||||
gen := server.configCache.providersGeneration()
|
||||
require.NoError(t, ps.Publish(coderdpubsub.AIProvidersChangedChannel, nil))
|
||||
|
||||
require.Eventually(t, func() bool {
|
||||
return server.configCache.providersGeneration() > gen
|
||||
}, testutil.WaitShort, testutil.IntervalFast)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user