From 41f815c0372225e558559d6d566e9f8261c26bcf Mon Sep 17 00:00:00 2001 From: Atef Bellaaj Date: Thu, 18 Dec 2025 20:06:13 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=20feat:=20Add=20Keyv=20memory=20cache?= =?UTF-8?q?=20read-through=20for=20MCPServersRegistry=20(#11030)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Atef Bellaaj --- packages/api/src/cache/cacheConfig.ts | 7 ++ .../src/mcp/registry/MCPServersRegistry.ts | 50 +++++++- ...PServersRegistry.cache_integration.spec.ts | 9 +- .../__tests__/MCPServersRegistry.test.ts | 116 +++++++++++++++++- 4 files changed, 171 insertions(+), 11 deletions(-) diff --git a/packages/api/src/cache/cacheConfig.ts b/packages/api/src/cache/cacheConfig.ts index 306ecb72d8..db4cc21921 100644 --- a/packages/api/src/cache/cacheConfig.ts +++ b/packages/api/src/cache/cacheConfig.ts @@ -112,6 +112,13 @@ const cacheConfig = { * @default 1000 */ REDIS_SCAN_COUNT: math(process.env.REDIS_SCAN_COUNT, 1000), + + /** + * TTL in milliseconds for MCP registry read-through cache. + * This cache reduces redundant lookups within a single request flow. + * @default 5000 (5 seconds) + */ + MCP_REGISTRY_CACHE_TTL: math(process.env.MCP_REGISTRY_CACHE_TTL, 5000), }; export { cacheConfig }; diff --git a/packages/api/src/mcp/registry/MCPServersRegistry.ts b/packages/api/src/mcp/registry/MCPServersRegistry.ts index 9c097270b5..54b62c3ff9 100644 --- a/packages/api/src/mcp/registry/MCPServersRegistry.ts +++ b/packages/api/src/mcp/registry/MCPServersRegistry.ts @@ -1,3 +1,4 @@ +import { Keyv } from 'keyv'; import { logger } from '@librechat/data-schemas'; import type { IServerConfigsRepositoryInterface } from './ServerConfigsRepositoryInterface'; import type * as t from '~/mcp/types'; @@ -5,6 +6,7 @@ import { MCPInspectionFailedError, isMCPDomainNotAllowedError } from '~/mcp/erro import { ServerConfigsCacheFactory } from './cache/ServerConfigsCacheFactory'; import { MCPServerInspector } from './MCPServerInspector'; import { ServerConfigsDB } from './db/ServerConfigsDB'; +import { cacheConfig } from '~/cache/cacheConfig'; /** * Central registry for managing MCP server configurations. @@ -22,11 +24,25 @@ export class MCPServersRegistry { private readonly dbConfigsRepo: IServerConfigsRepositoryInterface; private readonly cacheConfigsRepo: IServerConfigsRepositoryInterface; private readonly allowedDomains?: string[] | null; + private readonly readThroughCache: Keyv; + private readonly readThroughCacheAll: Keyv>; constructor(mongoose: typeof import('mongoose'), allowedDomains?: string[] | null) { this.dbConfigsRepo = new ServerConfigsDB(mongoose); this.cacheConfigsRepo = ServerConfigsCacheFactory.create('App', false); this.allowedDomains = allowedDomains; + + const ttl = cacheConfig.MCP_REGISTRY_CACHE_TTL; + + this.readThroughCache = new Keyv({ + namespace: 'mcp-registry-read-through', + ttl, + }); + + this.readThroughCacheAll = new Keyv>({ + namespace: 'mcp-registry-read-through-all', + ttl, + }); } /** Creates and initializes the singleton MCPServersRegistry instance */ @@ -61,20 +77,40 @@ export class MCPServersRegistry { serverName: string, userId?: string, ): Promise { + const cacheKey = this.getReadThroughCacheKey(serverName, userId); + + if (await this.readThroughCache.has(cacheKey)) { + return await this.readThroughCache.get(cacheKey); + } + // First we check if any config exist with the cache // Yaml config are pre loaded to the cache const configFromCache = await this.cacheConfigsRepo.get(serverName); - if (configFromCache) return configFromCache; + if (configFromCache) { + await this.readThroughCache.set(cacheKey, configFromCache); + return configFromCache; + } + const configFromDB = await this.dbConfigsRepo.get(serverName, userId); - if (configFromDB) return configFromDB; - return undefined; + await this.readThroughCache.set(cacheKey, configFromDB); + return configFromDB; } public async getAllServerConfigs(userId?: string): Promise> { - return { + const cacheKey = userId ?? '__no_user__'; + + // Check if key exists in read-through cache + if (await this.readThroughCacheAll.has(cacheKey)) { + return (await this.readThroughCacheAll.get(cacheKey)) ?? {}; + } + + const result = { ...(await this.cacheConfigsRepo.getAll()), ...(await this.dbConfigsRepo.getAll(userId)), }; + + await this.readThroughCacheAll.set(cacheKey, result); + return result; } public async addServer( @@ -156,6 +192,8 @@ export class MCPServersRegistry { public async reset(): Promise { await this.cacheConfigsRepo.reset(); + await this.readThroughCache.clear(); + await this.readThroughCacheAll.clear(); } public async removeServer( @@ -179,4 +217,8 @@ export class MCPServersRegistry { ); } } + + private getReadThroughCacheKey(serverName: string, userId?: string): string { + return userId ? `${serverName}::${userId}` : serverName; + } } diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.cache_integration.spec.ts b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.cache_integration.spec.ts index c2a3a0ae09..d20092c962 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.cache_integration.spec.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.cache_integration.spec.ts @@ -192,15 +192,14 @@ describe('MCPServersRegistry Redis Integration Tests', () => { // Add server await registry.addServer(serverName, testRawConfig, 'CACHE'); - // Verify server exists - const configBefore = await registry.getServerConfig(serverName); - expect(configBefore).toBeDefined(); + // Verify server exists in underlying cache repository (not via getServerConfig to avoid populating read-through cache) + expect(await registry['cacheConfigsRepo'].get(serverName)).toBeDefined(); // Remove server await registry.removeServer(serverName, 'CACHE'); - // Verify server was removed - const configAfter = await registry.getServerConfig(serverName); + // Verify server was removed from underlying cache repository + const configAfter = await registry['cacheConfigsRepo'].get(serverName); expect(configAfter).toBeUndefined(); }); }); diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts index 9db43c4f87..cc86f0e140 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts @@ -158,11 +158,13 @@ describe('MCPServersRegistry', () => { it('should route removeServer to cache repository', async () => { await registry.addServer('cache_server', testParsedConfig, 'CACHE'); - expect(await registry.getServerConfig('cache_server')).toBeDefined(); + // Verify server exists in underlying cache repository (not via getServerConfig to avoid populating read-through cache) + expect(await registry['cacheConfigsRepo'].get('cache_server')).toBeDefined(); await registry.removeServer('cache_server', 'CACHE'); - const config = await registry.getServerConfig('cache_server'); + // Verify server is removed from underlying cache repository + const config = await registry['cacheConfigsRepo'].get('cache_server'); expect(config).toBeUndefined(); }); }); @@ -190,4 +192,114 @@ describe('MCPServersRegistry', () => { }); }); }); + + describe('Read-through cache', () => { + describe('getServerConfig', () => { + it('should cache repeated calls for the same server', async () => { + // Add a server to the cache repository + await registry['cacheConfigsRepo'].add('test_server', testParsedConfig); + + // Spy on the cache repository get method + const cacheRepoGetSpy = jest.spyOn(registry['cacheConfigsRepo'], 'get'); + + // First call should hit the cache repository + const config1 = await registry.getServerConfig('test_server'); + expect(config1).toEqual(testParsedConfig); + expect(cacheRepoGetSpy).toHaveBeenCalledTimes(1); + + // Second call should hit the read-through cache, not the repository + const config2 = await registry.getServerConfig('test_server'); + expect(config2).toEqual(testParsedConfig); + expect(cacheRepoGetSpy).toHaveBeenCalledTimes(1); // Still 1, not 2 + + // Third call should also hit the read-through cache + const config3 = await registry.getServerConfig('test_server'); + expect(config3).toEqual(testParsedConfig); + expect(cacheRepoGetSpy).toHaveBeenCalledTimes(1); // Still 1 + }); + + it('should cache "not found" results to avoid repeated DB lookups', async () => { + // Spy on the DB repository get method + const dbRepoGetSpy = jest.spyOn(registry['dbConfigsRepo'], 'get'); + + // First call - server doesn't exist, should hit DB + const config1 = await registry.getServerConfig('nonexistent_server'); + expect(config1).toBeUndefined(); + expect(dbRepoGetSpy).toHaveBeenCalledTimes(1); + + // Second call - should hit read-through cache, not DB + const config2 = await registry.getServerConfig('nonexistent_server'); + expect(config2).toBeUndefined(); + expect(dbRepoGetSpy).toHaveBeenCalledTimes(1); // Still 1, not 2 + }); + + it('should use different cache keys for different userIds', async () => { + // Spy on the cache repository get method + const cacheRepoGetSpy = jest.spyOn(registry['cacheConfigsRepo'], 'get'); + + // First call without userId + await registry.getServerConfig('test_server'); + expect(cacheRepoGetSpy).toHaveBeenCalledTimes(1); + + // Call with userId - should be a different cache key, so hits repository again + await registry.getServerConfig('test_server', 'user123'); + expect(cacheRepoGetSpy).toHaveBeenCalledTimes(2); + + // Repeat call with same userId - should hit read-through cache + await registry.getServerConfig('test_server', 'user123'); + expect(cacheRepoGetSpy).toHaveBeenCalledTimes(2); // Still 2 + + // Call with different userId - should hit repository + await registry.getServerConfig('test_server', 'user456'); + expect(cacheRepoGetSpy).toHaveBeenCalledTimes(3); + }); + }); + + describe('getAllServerConfigs', () => { + it('should cache repeated calls', async () => { + // Add servers to cache + await registry['cacheConfigsRepo'].add('server1', testParsedConfig); + await registry['cacheConfigsRepo'].add('server2', testParsedConfig); + + // Spy on the cache repository getAll method + const cacheRepoGetAllSpy = jest.spyOn(registry['cacheConfigsRepo'], 'getAll'); + + // First call should hit the repository + const configs1 = await registry.getAllServerConfigs(); + expect(Object.keys(configs1)).toHaveLength(2); + expect(cacheRepoGetAllSpy).toHaveBeenCalledTimes(1); + + // Second call should hit the read-through cache + const configs2 = await registry.getAllServerConfigs(); + expect(Object.keys(configs2)).toHaveLength(2); + expect(cacheRepoGetAllSpy).toHaveBeenCalledTimes(1); // Still 1 + + // Third call should also hit the read-through cache + const configs3 = await registry.getAllServerConfigs(); + expect(Object.keys(configs3)).toHaveLength(2); + expect(cacheRepoGetAllSpy).toHaveBeenCalledTimes(1); // Still 1 + }); + + it('should use different cache keys for different userIds', async () => { + // Spy on the cache repository getAll method + const cacheRepoGetAllSpy = jest.spyOn(registry['cacheConfigsRepo'], 'getAll'); + + // First call without userId + await registry.getAllServerConfigs(); + expect(cacheRepoGetAllSpy).toHaveBeenCalledTimes(1); + + // Call with userId - should be a different cache key + await registry.getAllServerConfigs('user123'); + expect(cacheRepoGetAllSpy).toHaveBeenCalledTimes(2); + + // Repeat call with same userId - should hit read-through cache + await registry.getAllServerConfigs('user123'); + expect(cacheRepoGetAllSpy).toHaveBeenCalledTimes(2); // Still 2 + + // Call with different userId - should hit repository + await registry.getAllServerConfigs('user456'); + expect(cacheRepoGetAllSpy).toHaveBeenCalledTimes(3); + }); + }); + }); });