Skip to content

Commit 85ede8b

Browse files
committed
fix: eliminate per-request plugin manager rebuilds (#6532)
Two root causes drove the plugin manager (including the external OPA rules plugin) to re-initialize on nearly every request: Root cause A — DEFAULT_CACHE_TTL=30 forced a full manager rebuild every 30 seconds under sustained traffic. Each rebuild called manager.initialize() which re-imports and re-instantiates every configured plugin. Root cause B — per-tool context_id fragmentation created one cache slot per (team_id, tool_name) pair, triggering O(N tools) cold-start rebuilds. Fix A: set DEFAULT_CACHE_TTL=0 so managers live for the worker lifetime. Add PLUGIN_MANAGER_CACHE_TTL config knob (default 0) so operators can re-enable timed refresh when needed. Explicit invalidation signals (Redis pub/sub mode_change, binding_change, team_binding_change) already handle config propagation; TTL-driven expiry was a blunt workaround. Fix B: introduce TEAM_SCOPED_TOOL_SENTINEL ('__team__'). When _build_manager finds no per-tool ToolPluginBinding rows, it normalises the cache key from '<team_id>::<tool_name>' to '<team_id>::__team__'. get_manager checks the canonical team-scoped slot before spawning a new build task. All unbound tools in the same team share one manager instance instead of each creating their own slot. Tools with explicit bindings retain their per-tool key and dedicated manager. Security invariant preserved: the manager holds no per-request state. Per-request identity and auth context are passed as hook call arguments on every invocation; manager lifetime does not affect cross-tenant isolation. Closes #6532 Signed-off-by: Lang-Akshay <akshay.shinde26@ibm.com>
1 parent a211ab7 commit 85ede8b

6 files changed

Lines changed: 284 additions & 11 deletions

File tree

‎mcpgateway/config.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1392,6 +1392,17 @@ def normalize_x_frame_options(cls, v: Optional[str]) -> Optional[str]:
13921392
"Requires a server restart to take effect."
13931393
),
13941394
)
1395+
plugin_manager_cache_ttl: int = Field(
1396+
default=0,
1397+
ge=0,
1398+
description=(
1399+
"TTL in seconds for per-context plugin manager cache entries. "
1400+
"0 (default) disables expiry so managers live for the worker lifetime and are refreshed "
1401+
"only via explicit invalidation signals (Redis pub/sub, binding changes). "
1402+
"Set to a positive integer only when you need periodic config refresh and "
1403+
"accept the per-TTL re-initialization cost for plugins such as the external OPA rules plugin."
1404+
),
1405+
)
13951406

13961407
# Values used to detect unconfigured or insecure deployment states
13971408
SENTINEL_VALUES: ClassVar[list[str]] = ["", "UNCONFIGURED"]

‎mcpgateway/main.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1592,6 +1592,7 @@ def _notification_handler_factory(url: str, gateway_id, *, downstream_session_id
15921592
hook_policies=HOOK_PAYLOAD_POLICIES,
15931593
observability=None, # Will be set later if needed
15941594
db_factory=SessionLocal,
1595+
cache_ttl=settings.plugin_manager_cache_ttl,
15951596
)
15961597
logger.info("Plugin manager factory initialized")
15971598
except Exception as init_exc:

‎mcpgateway/plugins/__init__.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@ def init_plugin_manager_factory(
307307
hook_policies: dict,
308308
observability: Optional[ObservabilityProvider] = None,
309309
db_factory: Optional[Callable] = None,
310+
cache_ttl: Optional[int] = None,
310311
) -> None:
311312
"""Explicitly initialise the global plugin manager factory."""
312313
global _plugin_manager_factory
@@ -321,6 +322,7 @@ def init_plugin_manager_factory(
321322
hook_policies=hook_policies,
322323
observability=observability,
323324
db_factory=db_factory,
325+
cache_ttl=cache_ttl,
324326
)
325327

326328

‎mcpgateway/plugins/gateway_plugin_manager.py‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@
3939
logger = logging.getLogger(__name__)
4040

4141
CONTEXT_ID_SEPARATOR = "::"
42+
# Sentinel tool-name used as the shared cache slot for team-scoped tools that
43+
# have no per-tool ToolPluginBinding rows. All tools in a team without explicit
44+
# bindings resolve to "<team_id>::__team__" so they share one manager instance
45+
# instead of each creating their own slot.
46+
TEAM_SCOPED_TOOL_SENTINEL = "__team__"
4247

4348
_LEGACY_MODE_TO_PLUGIN_MODE: dict[str, tuple[PluginMode, Optional[OnError]]] = {
4449
"enforce": (PluginMode.SEQUENTIAL, None),
@@ -86,7 +91,7 @@ class TenantPluginManagerFactory:
8691
Call sites should use :func:`make_context_id` to construct them.
8792
"""
8893

89-
DEFAULT_CACHE_TTL = 30
94+
DEFAULT_CACHE_TTL = 0 # 0 = no expiry; managers live for the worker lifetime
9095
CONTEXT_ID_SEPARATOR = CONTEXT_ID_SEPARATOR
9196

9297
def __init__(
@@ -127,7 +132,16 @@ def plugin_names(self) -> list[str]:
127132
return [plugin.name for plugin in self._base_config.plugins]
128133

129134
async def get_manager(self, context_id: Optional[str] = None) -> TenantPluginManager:
130-
"""Get or create a TenantPluginManager for the given context."""
135+
"""Get or create a TenantPluginManager for the given context.
136+
137+
For per-tool context IDs (``"<team_id>::<tool_name>"``), the factory
138+
first checks whether a team-scoped canonical slot
139+
(``"<team_id>::__team__"``) already exists. When a tool has no
140+
per-tool ``ToolPluginBinding`` rows, ``_build_manager`` normalises the
141+
key to the team-scoped slot before caching, so all unbound tools in the
142+
same team share one manager instance. The per-tool key is looked up
143+
first so that tools *with* bindings still get their own dedicated slot.
144+
"""
131145
context_id = context_id or "__global__"
132146
expired = None
133147

@@ -140,6 +154,17 @@ async def get_manager(self, context_id: Optional[str] = None) -> TenantPluginMan
140154
else:
141155
return entry.manager
142156

157+
# Fast path: check the canonical team-scoped slot for per-tool keys.
158+
# _build_manager stores unbound tools under "<team_id>::__team__"
159+
# so a second unbound tool in the same team hits the cache here.
160+
if CONTEXT_ID_SEPARATOR in context_id:
161+
team_id, tool_name = context_id.split(CONTEXT_ID_SEPARATOR, 1)
162+
if tool_name != TEAM_SCOPED_TOOL_SENTINEL:
163+
canonical = f"{team_id}{CONTEXT_ID_SEPARATOR}{TEAM_SCOPED_TOOL_SENTINEL}"
164+
canonical_entry = self._managers.get(canonical)
165+
if canonical_entry is not None and not canonical_entry.is_expired(self._cache_ttl):
166+
return canonical_entry.manager
167+
143168
if expired is not None:
144169
try:
145170
await expired.manager.shutdown()
@@ -164,10 +189,33 @@ async def get_manager(self, context_id: Optional[str] = None) -> TenantPluginMan
164189
self._inflight.pop(context_id, None)
165190

166191
async def _build_manager(self, context_id: str) -> TenantPluginManager:
167-
"""Create, initialise, and cache a new manager for *context_id*."""
192+
"""Create, initialise, and cache a new manager for *context_id*.
193+
194+
When *context_id* is a per-tool key (``"<team_id>::<tool_name>"``) and
195+
``get_config_from_db`` finds no per-tool bindings, the manager is stored
196+
under the canonical team-scoped key ``"<team_id>::__team__"`` instead.
197+
Subsequent calls for other unbound tools in the same team hit the
198+
team-scoped cache entry in ``get_manager`` without rebuilding.
199+
"""
168200
manager = None
169201
try:
170202
new_config = await self.get_config_from_db(context_id)
203+
204+
# Normalize unbound per-tool keys to the team-scoped canonical slot.
205+
# A tool with no ToolPluginBinding rows produces the same manager as
206+
# any other unbound tool in its team; sharing one slot avoids O(tools)
207+
# cold-start rebuilds.
208+
cache_key = context_id
209+
if new_config is None and CONTEXT_ID_SEPARATOR in context_id:
210+
team_id, tool_name = context_id.split(CONTEXT_ID_SEPARATOR, 1)
211+
if tool_name != TEAM_SCOPED_TOOL_SENTINEL:
212+
cache_key = f"{team_id}{CONTEXT_ID_SEPARATOR}{TEAM_SCOPED_TOOL_SENTINEL}"
213+
async with self._lock:
214+
existing = self._managers.get(cache_key)
215+
if existing is not None and not existing.is_expired(self._cache_ttl):
216+
logger.debug("_build_manager: reusing team-scoped slot %s for %s", cache_key, context_id)
217+
return existing.manager
218+
171219
config = self._merge_tenant_config(new_config)
172220
config = await self._apply_redis_mode_overrides(config)
173221

@@ -180,15 +228,15 @@ async def _build_manager(self, context_id: str) -> TenantPluginManager:
180228
await manager.initialize()
181229

182230
async with self._lock:
183-
old_entry = self._managers.get(context_id)
184-
self._managers[context_id] = _CachedManager(manager=manager, created_at=time.monotonic())
231+
old_entry = self._managers.get(cache_key)
232+
self._managers[cache_key] = _CachedManager(manager=manager, created_at=time.monotonic())
185233

186234
old = old_entry.manager if old_entry is not None else None
187235
if old is not None and old is not manager:
188236
try:
189237
await old.shutdown()
190238
except Exception:
191-
logger.warning("Failed to shutdown old manager for context_id=%s", context_id)
239+
logger.warning("Failed to shutdown old manager for context_id=%s", cache_key)
192240

193241
return manager
194242

0 commit comments

Comments
 (0)