Skip to content

Commit 4aa4d97

Browse files
phernandezclaude
andcommitted
fix(core): reuse a single embedding provider per process
Defensive hardening for a reported 25+ GB memory leak in long-running 'basic-memory mcp' servers (Refs #872). onnxruntime's CPU memory arena never returns memory to the OS, so any duplicate FastEmbed/ONNX model load leaks tens of GB. Three independent fixes, each shippable on its own: 1. Inject one cached provider into every search repository. The create_search_repository factory now resolves a single provider via create_embedding_provider() and passes it into the SQLite/Postgres repos (the embedding_provider param already existed but no caller used it). This covers the per-request, per-sync, and watcher paths, which all route through the factory. Behavior is unchanged when semantic search is disabled. 2. Make the provider cache key stable. The key previously included CPU-count-derived thread/parallel knobs, which can drift between calls in a container/cgroup and silently mint a fresh key -> a fresh model load. The key now contains only model-identity fields; thread/parallel knobs are still applied to the provider, just not part of the key. 3. Disable onnxruntime's CPU memory arena. FastEmbed 0.8.0 exposes enable_cpu_mem_arena via TextEmbedding session-option kwargs, so the FastEmbed provider now passes enable_cpu_mem_arena=False to let any transient extra load return memory to the OS. Also logs a WARNING when a second distinct provider cache key is created in one process, so future cache bypasses are visible. Root-cause confirmation is still pending the reporter's logs (already requested on the issue); these changes are defensive and safe to ship regardless. Refs #872 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 1667cdc commit 4aa4d97

6 files changed

Lines changed: 349 additions & 21 deletions

File tree

src/basic_memory/repository/embedding_provider_factory.py

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,19 @@
33
import os
44
from threading import Lock
55

6+
from loguru import logger
7+
68
from basic_memory.config import BasicMemoryConfig, default_fastembed_cache_dir
79
from basic_memory.repository.embedding_provider import EmbeddingProvider
810

11+
# Cache key fields are limited to values that change the *identity* of the loaded
12+
# model (provider, model_name, dimensions, LiteLLM role/input-type/forward-dimension
13+
# settings, batch/request knobs that affect the LiteLLM identity, and the resolved
14+
# cache dir). Thread/parallel knobs are deliberately excluded — they change ONNX
15+
# *execution* only, not the loaded weights. Including them caused #872: in a
16+
# container/cgroup the CPU-derived thread count can drift between calls, producing
17+
# a fresh cache key and reloading the ~2.3GB model into a CPU arena that never
18+
# returns memory to the OS.
919
type ProviderCacheKey = tuple[
1020
str,
1121
str,
@@ -16,8 +26,6 @@
1626
str | None,
1727
str | None,
1828
str,
19-
int | None,
20-
int | None,
2129
]
2230

2331
_EMBEDDING_PROVIDER_CACHE: dict[ProviderCacheKey, EmbeddingProvider] = {}
@@ -78,13 +86,16 @@ def _resolve_fastembed_runtime_knobs(
7886

7987

8088
def _provider_cache_key(app_config: BasicMemoryConfig) -> ProviderCacheKey:
81-
"""Build a stable cache key from provider-relevant semantic embedding config.
89+
"""Build a stable cache key from model-identity semantic embedding config.
8290
8391
Uses the *resolved* cache dir — not the raw config field — so different
8492
FASTEMBED_CACHE_PATH values produce distinct cache keys even when the
8593
config field itself is unset.
94+
95+
Deliberately excludes the FastEmbed thread/parallel knobs: they tune ONNX
96+
execution, not which model weights are loaded, and resolving them from the
97+
runtime CPU budget makes the key drift between calls in a container (#872).
8698
"""
87-
resolved_threads, resolved_parallel = _resolve_fastembed_runtime_knobs(app_config)
8899
return (
89100
app_config.semantic_embedding_provider.strip().lower(),
90101
app_config.semantic_embedding_model,
@@ -95,8 +106,6 @@ def _provider_cache_key(app_config: BasicMemoryConfig) -> ProviderCacheKey:
95106
app_config.semantic_embedding_document_input_type,
96107
app_config.semantic_embedding_query_input_type,
97108
_resolve_cache_dir(app_config),
98-
resolved_threads,
99-
resolved_parallel,
100109
)
101110

102111

@@ -190,5 +199,19 @@ def create_embedding_provider(app_config: BasicMemoryConfig) -> EmbeddingProvide
190199
with _EMBEDDING_PROVIDER_CACHE_LOCK:
191200
if cached_provider := _EMBEDDING_PROVIDER_CACHE.get(cache_key):
192201
return cached_provider
202+
# Trigger: a distinct cache key is being inserted while the cache already
203+
# holds entries for other keys.
204+
# Why: the provider is meant to be a process-wide singleton (#872). A second
205+
# key means something bypassed reuse — a real config change, or a regression
206+
# that reintroduces volatile fields into the key — and each new key reloads
207+
# the ~2.3GB ONNX model into a CPU arena that never releases memory.
208+
# Outcome: surface the bypass so future leaks are diagnosable from logs.
209+
if _EMBEDDING_PROVIDER_CACHE:
210+
logger.warning(
211+
"Creating a second distinct embedding provider in this process; "
212+
"the model will be loaded again. existing_keys={existing} new_key={new}",
213+
existing=list(_EMBEDDING_PROVIDER_CACHE.keys()),
214+
new=cache_key,
215+
)
193216
_EMBEDDING_PROVIDER_CACHE[cache_key] = provider
194217
return provider

src/basic_memory/repository/fastembed_provider.py

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,20 @@ def _create_model() -> "TextEmbedding":
7373
"pip install -U basic-memory"
7474
) from exc
7575
resolved_model_name = self._MODEL_ALIASES.get(self.model_name, self.model_name)
76-
if self.cache_dir is not None and self.threads is not None:
77-
return TextEmbedding(
78-
model_name=resolved_model_name,
79-
cache_dir=self.cache_dir,
80-
threads=self.threads,
81-
)
76+
# Constraint: onnxruntime's CPU memory arena grows to fit peak usage
77+
# and never returns that memory to the OS. If a model is ever loaded
78+
# more than once in a long-running process it leaks tens of GB (#872).
79+
# FastEmbed exposes enable_cpu_mem_arena via its session-option kwargs,
80+
# so we disable the arena to let any transient extra load free memory.
81+
model_kwargs: dict = {
82+
"model_name": resolved_model_name,
83+
"enable_cpu_mem_arena": False,
84+
}
8285
if self.cache_dir is not None:
83-
return TextEmbedding(model_name=resolved_model_name, cache_dir=self.cache_dir)
86+
model_kwargs["cache_dir"] = self.cache_dir
8487
if self.threads is not None:
85-
return TextEmbedding(model_name=resolved_model_name, threads=self.threads)
86-
return TextEmbedding(model_name=resolved_model_name)
88+
model_kwargs["threads"] = self.threads
89+
return TextEmbedding(**model_kwargs)
8790

8891
self._model = await asyncio.to_thread(_create_model)
8992
logger.info(

src/basic_memory/repository/search_repository.py

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
1414

1515
from basic_memory.config import BasicMemoryConfig, ConfigManager, DatabaseBackend
16+
from basic_memory.repository.embedding_provider_factory import create_embedding_provider
1617
from basic_memory.repository.postgres_search_repository import PostgresSearchRepository
1718
from basic_memory.repository.search_index_row import SearchIndexRow
1819
from basic_memory.repository.search_repository_base import VectorSyncBatchResult
@@ -120,19 +121,37 @@ def create_search_repository(
120121
Returns:
121122
SearchRepository: Backend-appropriate search repository instance
122123
"""
123-
# Prefer explicit parameter; fall back to ConfigManager for backwards compatibility
124+
# Resolve config once so backend detection and the shared embedding provider
125+
# come from the same source. Prefer the explicit arg; fall back to ConfigManager
126+
# for backwards compatibility.
127+
config = app_config or ConfigManager().config
124128
if database_backend is None:
125-
config = app_config or ConfigManager().config
126129
database_backend = config.database_backend
127130

131+
# Trigger: every request, sync batch, and project builds its own search repo.
132+
# Why: each repo __init__ would otherwise call create_embedding_provider(), and
133+
# the process-wide cache can be bypassed if its key ever drifts (#872), reloading
134+
# the ~2.3GB ONNX model and leaking memory in onnxruntime's CPU arena.
135+
# Outcome: resolve the cached singleton here once and inject it, so the provider
136+
# is the single source of truth across all callers of this factory.
137+
embedding_provider = None
138+
if config.semantic_search_enabled:
139+
embedding_provider = create_embedding_provider(config)
140+
128141
if database_backend == DatabaseBackend.POSTGRES: # pragma: no cover
129142
return PostgresSearchRepository( # pragma: no cover
130143
session_maker,
131144
project_id=project_id,
132145
app_config=app_config,
146+
embedding_provider=embedding_provider,
133147
)
134148
else:
135-
return SQLiteSearchRepository(session_maker, project_id=project_id, app_config=app_config)
149+
return SQLiteSearchRepository(
150+
session_maker,
151+
project_id=project_id,
152+
app_config=app_config,
153+
embedding_provider=embedding_provider,
154+
)
136155

137156

138157
__all__ = [
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
"""Integration tests for process-wide embedding provider reuse (#872).
2+
3+
A long-running ``basic-memory mcp`` server constructs a new search repository per
4+
request, per sync batch, and per project. Each repository used to derive its own
5+
embedding provider via ``create_embedding_provider()``. If the provider cache key
6+
ever drifted, that reloaded the ~2.3GB FastEmbed/ONNX model and leaked memory in
7+
onnxruntime's CPU arena (which never returns memory to the OS).
8+
9+
These tests use the *real* composition paths — ``create_embedding_provider``, the
10+
``create_search_repository`` factory, and the FastAPI deps function
11+
``get_search_repository`` — with a real FastEmbed provider. FastEmbed loads the
12+
ONNX model lazily on first embed, so constructing providers/repositories here is
13+
cheap and never touches the native model.
14+
"""
15+
16+
from __future__ import annotations
17+
18+
from typing import cast
19+
20+
import pytest
21+
22+
from basic_memory.config import BasicMemoryConfig, DatabaseBackend, ProjectEntry
23+
from basic_memory.deps.repositories import get_search_repository
24+
from basic_memory.repository.embedding_provider_factory import (
25+
create_embedding_provider,
26+
reset_embedding_provider_cache,
27+
)
28+
from basic_memory.repository.fastembed_provider import FastEmbedEmbeddingProvider
29+
from basic_memory.repository.search_repository import create_search_repository
30+
from basic_memory.repository.sqlite_search_repository import SQLiteSearchRepository
31+
32+
33+
def _semantic_config(config_home) -> BasicMemoryConfig:
34+
"""Build a semantic-enabled FastEmbed config rooted at the test home."""
35+
return BasicMemoryConfig(
36+
env="test",
37+
projects={"test-project": ProjectEntry(path=str(config_home))},
38+
default_project="test-project",
39+
database_backend=DatabaseBackend.SQLITE,
40+
semantic_search_enabled=True,
41+
semantic_embedding_provider="fastembed",
42+
)
43+
44+
45+
@pytest.fixture(autouse=True)
46+
def _reset_provider_cache():
47+
reset_embedding_provider_cache()
48+
yield
49+
reset_embedding_provider_cache()
50+
51+
52+
def test_factory_resolves_single_provider_across_repositories(config_home, engine_factory):
53+
"""The factory must inject one cached provider into every search repository."""
54+
_engine, session_maker = engine_factory
55+
config = _semantic_config(config_home)
56+
57+
expected_provider = create_embedding_provider(config)
58+
assert isinstance(expected_provider, FastEmbedEmbeddingProvider)
59+
60+
# Two repositories, mimicking per-request / per-sync construction.
61+
repo_a = cast(
62+
SQLiteSearchRepository,
63+
create_search_repository(session_maker, project_id=1, app_config=config),
64+
)
65+
repo_b = cast(
66+
SQLiteSearchRepository,
67+
create_search_repository(session_maker, project_id=2, app_config=config),
68+
)
69+
70+
# Both repos reuse the exact same cached provider object — no second model load.
71+
assert repo_a._embedding_provider is expected_provider
72+
assert repo_b._embedding_provider is expected_provider
73+
74+
75+
@pytest.mark.asyncio
76+
async def test_deps_path_reuses_cached_provider(config_home, engine_factory):
77+
"""The real FastAPI deps function must reuse the cached provider, not rebuild it."""
78+
_engine, session_maker = engine_factory
79+
config = _semantic_config(config_home)
80+
81+
expected_provider = create_embedding_provider(config)
82+
83+
repo = cast(
84+
SQLiteSearchRepository,
85+
await get_search_repository(
86+
session_maker=session_maker,
87+
project_id=1,
88+
app_config=config,
89+
),
90+
)
91+
92+
assert repo._embedding_provider is expected_provider
93+
94+
95+
def test_factory_skips_provider_when_semantic_disabled(config_home, engine_factory):
96+
"""With semantic search off, no provider is created and none is injected."""
97+
_engine, session_maker = engine_factory
98+
config = BasicMemoryConfig(
99+
env="test",
100+
projects={"test-project": ProjectEntry(path=str(config_home))},
101+
default_project="test-project",
102+
database_backend=DatabaseBackend.SQLITE,
103+
semantic_search_enabled=False,
104+
)
105+
106+
repo = cast(
107+
SQLiteSearchRepository,
108+
create_search_repository(session_maker, project_id=1, app_config=config),
109+
)
110+
111+
assert repo._embedding_provider is None

0 commit comments

Comments
 (0)