{{ message }}
Repository navigation
fix(rag): share one index across agents referencing the same RAG - #4650
Merged
Merged
Conversation
VectorStore.needsIndexing only consulted the store's in-memory hash map, which tracks that store's own writes. When several stores share one database (e.g. many agents referencing the same RAG definition), a file re-indexed and committed by one store was still seen as stale by every other store, so each of them embedded it again. On an in-memory miss, fall back to the database metadata: if the stored hash matches the file and at least one chunk exists, record the hash locally and skip indexing. A metadata read error still re-indexes, so a transient database error cannot leave a file stale. Refs docker#4637
Every `type: rag` toolset built its own manager, so N agents referencing the same RAG definition ran N indexes, N file watchers and N embedding queues over one database. A single file change was embedded N times at once, multiplying the load on the embedding endpoint by N. References built from identical inputs now share one started toolset. The key fingerprints everything that builds the manager (RAG name, RAG config, canonical parent directory, gateway, models and providers) and compares the env provider and provider registry by identity, so an embedding client is never shared across different credentials. Each reference still builds its own toolset at load time. On Start the first reference installs it and later ones attach; queries and events go through the started toolset. The last Stop closes it and removes it from the registry, and a toolset that never starts never enters it. A failed start leaves nothing behind, so a later reference retries. Refs docker#4637
A shared RAG toolset's file watcher and indexing run on a context derived from the Start call, which drops cancellation but keeps values. The session ID of whichever session started the index therefore stayed on every background embedding request, and gateway-bound calls forwarded it as X-Cagent-Session-Id, even after that session had ended and while the index served other sessions. Clear the session ID before starting a shared index, keeping the context's logging and tracing values. Queries still run on each caller's own request context and carry its session. Refs docker#4637
Load a config where three agents reference the same RAG definition through the real team loader, toolset registry, sqlite database and file watcher, with a fake embedding provider counting inputs. Initial indexing and re-indexing after a file change must each embed the document once, not once per agent. Refs docker#4637
Document that references to one knowledge base share a single index, file watcher and embedding budget, when they stop sharing (a name: override or different credentials), and that shared background indexing is not attributed to a session. Refs docker#4637
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Path-key collisions, lost startup events and cached handlers targeting uninitialized managers remain unresolved.
3 open issues
What changed in this PR
Addresses #4637 by sharing RAG indexes across matching agent references, reducing duplicate indexing and embedding requests.
Changes:
- Adds reference-counted sharing with session-neutral background indexing.
- Checks database metadata before re-embedding files.
- Adds regression tests and sharing documentation.
Review effort: Balanced
Comment on lines
+42
to
+48
| dir := parentDir | ||
| if abs, err := filepath.Abs(dir); err == nil { | ||
| dir = abs | ||
| } | ||
| if resolved, err := filepath.EvalSymlinks(dir); err == nil { | ||
| dir = resolved | ||
| } |
| // Tools serves the toolset that was actually initialized: a reference that | ||
| // attached never indexed its own manager. | ||
| func (s *sharedToolSet) Tools(ctx context.Context) ([]tools.Tool, error) { | ||
| return s.target().Tools(ctx) |
Comment on lines
+213
to
+228
aheritier
added
area/docs
dgageot
approved these changes
Oct 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4637
Problem
Every
type: ragtoolset built its own RAG manager. With N agents referencing one RAG definition there were N vector stores, N file watchers, N debounce queues and Nmax_embedding_concurrencycaps over a single database. One file change was embedded N times at once, so the embedding endpoint sawN × concurrencysimultaneous requests (~43 re-index jobs per write in the reported setup, enough for the gateway to drop connections).A second defect made it worse:
VectorStore.needsIndexingonly read the store's in-memory hash map, so even after one store committed a file, every other store still saw it as stale and re-embedded it.Changes
pkg/rag/strategy). On an in-memory miss,needsIndexingconsults the database metadata and skips the file when its hash matches and chunks exist. A metadata read error still re-indexes, so a transient database error cannot leave a file stale.pkg/tools/builtin/rag/shared.go).CreateToolSetreturns a per-reference handle. OnStart, the first reference installs its toolset in a process-wide registry and later ones attach; queries and events go through the started toolset; the lastStopcloses it and removes the entry. Toolsets that never start never enter the registry, and a failed start leaves nothing behind so a later reference can retry.X-Cagent-Session-Idof whichever session happened to start it.Security considerations
The registry is process-wide, so in
serve apiit can span sessions. The key is designed so sharing never crosses a trust boundary:database:is unset.environment.Providervaries its values by context, so one instance means one set of credentials. Per-loadRuntimeConfig.Clone()keeps the instance;EnvProviderOverrideandCloneWithFreshEnvironment(e.g. ACP re-auth) produce a new one, which falls back to an unshared index, i.e. today's behaviour.Out of scope: references that override
name:keep their own index (the name feeds the default database path), and two different configs pointing at the samedatabase:file are not detected; both behave as before.Testing
TestSharedDB_SecondStoreSkipsFileAlreadyReindexed,TestNeedsIndexing_DBFallbackIgnoresZeroChunkMetadata: the database fallback.pkg/tools/builtin/rag/shared_test.go: identical keys index once, different or unshareable keys don't share, concurrent starts index once, failed starts are retried, events reach every reference, queries hit the initialized index,Toolsdoesn't block on indexing, background work carries no session, and key identity (credentials, name, directory, symlinks).TestRAGReferencesShareOneIndex: three agents loaded through the real team loader, sqlite and file watcher; initial indexing and a file change each embed once. With sharing disabled it fails with 2 embeddings for the change.go test -raceon the touched packages, golangci-lint v2.14.0, project cops and markdownlint are clean.