mirror of
https://github.com/danny-avila/LibreChat.git
synced 2025-12-20 10:20:15 +01:00
⚡ feat: Add Keyv memory cache read-through for MCPServersRegistry (#11030)
Co-authored-by: Atef Bellaaj <slalom.bellaaj@external.daimlertruck.com>
This commit is contained in:
parent
95a69df70e
commit
41f815c037
4 changed files with 171 additions and 11 deletions
7
packages/api/src/cache/cacheConfig.ts
vendored
7
packages/api/src/cache/cacheConfig.ts
vendored
|
|
@ -112,6 +112,13 @@ const cacheConfig = {
|
||||||
* @default 1000
|
* @default 1000
|
||||||
*/
|
*/
|
||||||
REDIS_SCAN_COUNT: math(process.env.REDIS_SCAN_COUNT, 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 };
|
export { cacheConfig };
|
||||||
|
|
|
||||||
|
|
@ -1,3 +1,4 @@
|
||||||
|
import { Keyv } from 'keyv';
|
||||||
import { logger } from '@librechat/data-schemas';
|
import { logger } from '@librechat/data-schemas';
|
||||||
import type { IServerConfigsRepositoryInterface } from './ServerConfigsRepositoryInterface';
|
import type { IServerConfigsRepositoryInterface } from './ServerConfigsRepositoryInterface';
|
||||||
import type * as t from '~/mcp/types';
|
import type * as t from '~/mcp/types';
|
||||||
|
|
@ -5,6 +6,7 @@ import { MCPInspectionFailedError, isMCPDomainNotAllowedError } from '~/mcp/erro
|
||||||
import { ServerConfigsCacheFactory } from './cache/ServerConfigsCacheFactory';
|
import { ServerConfigsCacheFactory } from './cache/ServerConfigsCacheFactory';
|
||||||
import { MCPServerInspector } from './MCPServerInspector';
|
import { MCPServerInspector } from './MCPServerInspector';
|
||||||
import { ServerConfigsDB } from './db/ServerConfigsDB';
|
import { ServerConfigsDB } from './db/ServerConfigsDB';
|
||||||
|
import { cacheConfig } from '~/cache/cacheConfig';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Central registry for managing MCP server configurations.
|
* Central registry for managing MCP server configurations.
|
||||||
|
|
@ -22,11 +24,25 @@ export class MCPServersRegistry {
|
||||||
private readonly dbConfigsRepo: IServerConfigsRepositoryInterface;
|
private readonly dbConfigsRepo: IServerConfigsRepositoryInterface;
|
||||||
private readonly cacheConfigsRepo: IServerConfigsRepositoryInterface;
|
private readonly cacheConfigsRepo: IServerConfigsRepositoryInterface;
|
||||||
private readonly allowedDomains?: string[] | null;
|
private readonly allowedDomains?: string[] | null;
|
||||||
|
private readonly readThroughCache: Keyv<t.ParsedServerConfig>;
|
||||||
|
private readonly readThroughCacheAll: Keyv<Record<string, t.ParsedServerConfig>>;
|
||||||
|
|
||||||
constructor(mongoose: typeof import('mongoose'), allowedDomains?: string[] | null) {
|
constructor(mongoose: typeof import('mongoose'), allowedDomains?: string[] | null) {
|
||||||
this.dbConfigsRepo = new ServerConfigsDB(mongoose);
|
this.dbConfigsRepo = new ServerConfigsDB(mongoose);
|
||||||
this.cacheConfigsRepo = ServerConfigsCacheFactory.create('App', false);
|
this.cacheConfigsRepo = ServerConfigsCacheFactory.create('App', false);
|
||||||
this.allowedDomains = allowedDomains;
|
this.allowedDomains = allowedDomains;
|
||||||
|
|
||||||
|
const ttl = cacheConfig.MCP_REGISTRY_CACHE_TTL;
|
||||||
|
|
||||||
|
this.readThroughCache = new Keyv<t.ParsedServerConfig>({
|
||||||
|
namespace: 'mcp-registry-read-through',
|
||||||
|
ttl,
|
||||||
|
});
|
||||||
|
|
||||||
|
this.readThroughCacheAll = new Keyv<Record<string, t.ParsedServerConfig>>({
|
||||||
|
namespace: 'mcp-registry-read-through-all',
|
||||||
|
ttl,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Creates and initializes the singleton MCPServersRegistry instance */
|
/** Creates and initializes the singleton MCPServersRegistry instance */
|
||||||
|
|
@ -61,20 +77,40 @@ export class MCPServersRegistry {
|
||||||
serverName: string,
|
serverName: string,
|
||||||
userId?: string,
|
userId?: string,
|
||||||
): Promise<t.ParsedServerConfig | undefined> {
|
): Promise<t.ParsedServerConfig | undefined> {
|
||||||
|
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
|
// First we check if any config exist with the cache
|
||||||
// Yaml config are pre loaded to the cache
|
// Yaml config are pre loaded to the cache
|
||||||
const configFromCache = await this.cacheConfigsRepo.get(serverName);
|
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);
|
const configFromDB = await this.dbConfigsRepo.get(serverName, userId);
|
||||||
if (configFromDB) return configFromDB;
|
await this.readThroughCache.set(cacheKey, configFromDB);
|
||||||
return undefined;
|
return configFromDB;
|
||||||
}
|
}
|
||||||
|
|
||||||
public async getAllServerConfigs(userId?: string): Promise<Record<string, t.ParsedServerConfig>> {
|
public async getAllServerConfigs(userId?: string): Promise<Record<string, t.ParsedServerConfig>> {
|
||||||
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.cacheConfigsRepo.getAll()),
|
||||||
...(await this.dbConfigsRepo.getAll(userId)),
|
...(await this.dbConfigsRepo.getAll(userId)),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
await this.readThroughCacheAll.set(cacheKey, result);
|
||||||
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
public async addServer(
|
public async addServer(
|
||||||
|
|
@ -156,6 +192,8 @@ export class MCPServersRegistry {
|
||||||
|
|
||||||
public async reset(): Promise<void> {
|
public async reset(): Promise<void> {
|
||||||
await this.cacheConfigsRepo.reset();
|
await this.cacheConfigsRepo.reset();
|
||||||
|
await this.readThroughCache.clear();
|
||||||
|
await this.readThroughCacheAll.clear();
|
||||||
}
|
}
|
||||||
|
|
||||||
public async removeServer(
|
public async removeServer(
|
||||||
|
|
@ -179,4 +217,8 @@ export class MCPServersRegistry {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private getReadThroughCacheKey(serverName: string, userId?: string): string {
|
||||||
|
return userId ? `${serverName}::${userId}` : serverName;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -192,15 +192,14 @@ describe('MCPServersRegistry Redis Integration Tests', () => {
|
||||||
// Add server
|
// Add server
|
||||||
await registry.addServer(serverName, testRawConfig, 'CACHE');
|
await registry.addServer(serverName, testRawConfig, 'CACHE');
|
||||||
|
|
||||||
// Verify server exists
|
// Verify server exists in underlying cache repository (not via getServerConfig to avoid populating read-through cache)
|
||||||
const configBefore = await registry.getServerConfig(serverName);
|
expect(await registry['cacheConfigsRepo'].get(serverName)).toBeDefined();
|
||||||
expect(configBefore).toBeDefined();
|
|
||||||
|
|
||||||
// Remove server
|
// Remove server
|
||||||
await registry.removeServer(serverName, 'CACHE');
|
await registry.removeServer(serverName, 'CACHE');
|
||||||
|
|
||||||
// Verify server was removed
|
// Verify server was removed from underlying cache repository
|
||||||
const configAfter = await registry.getServerConfig(serverName);
|
const configAfter = await registry['cacheConfigsRepo'].get(serverName);
|
||||||
expect(configAfter).toBeUndefined();
|
expect(configAfter).toBeUndefined();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -158,11 +158,13 @@ describe('MCPServersRegistry', () => {
|
||||||
|
|
||||||
it('should route removeServer to cache repository', async () => {
|
it('should route removeServer to cache repository', async () => {
|
||||||
await registry.addServer('cache_server', testParsedConfig, 'CACHE');
|
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');
|
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();
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue