feat(servers): per-server memory ceiling, because 512Mi OOMKills silently
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m16s
CI/CD / lint (pull_request) Successful in 2m16s
CI/CD / test (pull_request) Successful in 1m24s
CI/CD / build (pull_request) Successful in 2m10s
CI/CD / smoke (pull_request) Failing after 3m15s
CI/CD / publish (pull_request) Has been skipped
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m16s
CI/CD / lint (pull_request) Successful in 2m16s
CI/CD / test (pull_request) Successful in 1m24s
CI/CD / build (pull_request) Successful in 2m10s
CI/CD / smoke (pull_request) Failing after 3m15s
CI/CD / publish (pull_request) Has been skipped
Server pods have always had a hardcoded 512Mi limit. That is right for a server that proxies an API and fatal for one that drives a browser: the `docs` server (docs-mcp-server, which scrapes with headless Chromium) idles at ~228Mi and crosses 512Mi within seconds of its first scrape. An OOMKill is the quietest failure we have. The kernel kills it, the pod restarts, the readiness probe passes, and the instance reads `healthy` again — while the six scrape jobs whose queue lived in memory are gone and the index has 17 pages in it. Nothing is logged, because the process never got to say anything. memoryLimitMb is declared per server, in MiB, and converted to bytes in the container spec. NULL keeps DEFAULT_MEMORY_LIMIT, so every existing server is bit-for-bit unchanged — a bigger default would have cost real memory on every node for servers that do not need it. mcpctl create server docs --memory-limit-mb 2048 --force Tests assert the two places a new column dies quietly: the repository's field-by-field mapping (a column not mapped there returns "patched" and changes nothing) and the spec built in instance.service. docs/reliability gains the OOMKill section — the `lastState.terminated.reason` check is what turns "it restarted again" into an answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JPjtnE6Gd343oRNtMU9Bcd
This commit is contained in:
@@ -49,6 +49,7 @@ const ServerSpecSchema = z.object({
|
||||
// nullable: `get server -o yaml` emits null for servers with no override,
|
||||
// and that YAML must apply back unchanged.
|
||||
toolCallTimeoutSeconds: z.number().int().min(1).max(3600).nullable().optional(),
|
||||
memoryLimitMb: z.number().int().min(64).max(16384).nullable().optional(),
|
||||
});
|
||||
|
||||
const SecretSpecSchema = z.object({
|
||||
|
||||
@@ -252,6 +252,7 @@ export function createCreateCommand(deps: CreateCommandDeps): Command {
|
||||
.option('--health-check-interval <seconds>', 'Readiness probe interval in seconds (default 60)')
|
||||
.option('--health-check-timeout <seconds>', 'Readiness probe timeout in seconds (default 10)')
|
||||
.option('--tool-call-timeout <seconds>', 'Per-server tool-call deadline in seconds (default: mcplocal\'s global deadline)')
|
||||
.option('--memory-limit-mb <mb>', 'Per-server memory ceiling in MiB (default: 512). Raise it for a server that drives a browser or holds a large index — at the default it is OOMKilled with no error, just a restart')
|
||||
.option('--health-check-failure-threshold <count>', 'Consecutive failures before the instance is marked unhealthy (default 3)')
|
||||
.option('--secret-delivery <mode>', 'How secret env reaches the container: env (default, value written into the pod spec) or injector (pod fetches its own secrets from OpenBao under a scoped identity)')
|
||||
.option('--entrypoint <argv>', 'Comma-separated argv to exec under the injector wrapper. Required for --secret-delivery injector on a dockerImage server, whose ENTRYPOINT mcpd cannot introspect')
|
||||
@@ -335,6 +336,9 @@ export function createCreateCommand(deps: CreateCommandDeps): Command {
|
||||
if (opts.toolCallTimeout !== undefined) {
|
||||
body.toolCallTimeoutSeconds = parsePositiveInt('--tool-call-timeout', opts.toolCallTimeout as string);
|
||||
}
|
||||
if (opts.memoryLimitMb !== undefined) {
|
||||
body.memoryLimitMb = parsePositiveInt('--memory-limit-mb', opts.memoryLimitMb as string);
|
||||
}
|
||||
if (opts.packageName) body.packageName = opts.packageName;
|
||||
if (opts.runtime) body.runtime = opts.runtime;
|
||||
if (opts.dockerImage) body.dockerImage = opts.dockerImage;
|
||||
|
||||
@@ -0,0 +1,9 @@
|
||||
-- Per-server memory ceiling, in MiB.
|
||||
--
|
||||
-- NULL keeps today's behaviour exactly: the server gets the orchestrator's
|
||||
-- DEFAULT_MEMORY_LIMIT (512 MiB). That default is fine for a server that
|
||||
-- proxies an API, and fatal for one that drives a browser — docs-mcp-server
|
||||
-- scrapes with headless Chromium, sits at ~228 MiB idle, and crosses 512 MiB
|
||||
-- within seconds of the first scrape. The pod is OOMKilled and restarts, so
|
||||
-- the only trace is a restart count and an empty job queue.
|
||||
ALTER TABLE "McpServer" ADD COLUMN "memoryLimitMb" INTEGER;
|
||||
@@ -91,6 +91,13 @@ model McpServer {
|
||||
/// genuinely slow tools; the deadline exists so a wedged call answers instead
|
||||
/// of hanging, not to cut off honest work.
|
||||
toolCallTimeoutSeconds Int?
|
||||
|
||||
/// Per-server memory ceiling in MiB. Null = the orchestrator's 512 MiB
|
||||
/// default. Raise it for a server that legitimately needs more than that:
|
||||
/// docs-mcp-server scrapes with headless Chromium and is OOMKilled at the
|
||||
/// default before it indexes a single page, with nothing in the logs to say
|
||||
/// so — the pod simply restarts and the job queue is gone.
|
||||
memoryLimitMb Int?
|
||||
version Int @default(1)
|
||||
createdAt DateTime @default(now())
|
||||
updatedAt DateTime @updatedAt
|
||||
|
||||
@@ -37,6 +37,7 @@ export class McpServerRepository implements IMcpServerRepository {
|
||||
secretDelivery: data.secretDelivery,
|
||||
entrypoint: (data.entrypoint ?? Prisma.DbNull) as Prisma.InputJsonValue,
|
||||
toolCallTimeoutSeconds: data.toolCallTimeoutSeconds ?? null,
|
||||
memoryLimitMb: data.memoryLimitMb ?? null,
|
||||
},
|
||||
});
|
||||
}
|
||||
@@ -59,6 +60,7 @@ export class McpServerRepository implements IMcpServerRepository {
|
||||
if (data.secretDelivery !== undefined) updateData['secretDelivery'] = data.secretDelivery;
|
||||
if (data.entrypoint !== undefined) updateData['entrypoint'] = (data.entrypoint ?? Prisma.JsonNull) as Prisma.InputJsonValue;
|
||||
if (data.toolCallTimeoutSeconds !== undefined) updateData['toolCallTimeoutSeconds'] = data.toolCallTimeoutSeconds;
|
||||
if (data.memoryLimitMb !== undefined) updateData['memoryLimitMb'] = data.memoryLimitMb;
|
||||
|
||||
return this.prisma.mcpServer.update({ where: { id }, data: updateData });
|
||||
}
|
||||
|
||||
@@ -481,6 +481,16 @@ export class InstanceService {
|
||||
spec.containerPort = server.containerPort ?? 3000;
|
||||
}
|
||||
|
||||
// Per-server memory ceiling. Null leaves DEFAULT_MEMORY_LIMIT (512 MiB)
|
||||
// in place, which is right for an API proxy and fatal for a server that
|
||||
// drives a browser: docs-mcp-server was OOMKilled seconds into every
|
||||
// scrape, and an OOMKill leaves no error — the pod restarts, the
|
||||
// in-memory job queue is gone, and the instance reports healthy again.
|
||||
const memoryLimitMb = server.memoryLimitMb as number | null;
|
||||
if (memoryLimitMb !== null && memoryLimitMb !== undefined) {
|
||||
spec.memoryLimit = memoryLimitMb * 1024 * 1024;
|
||||
}
|
||||
|
||||
// Volumes are keyed on the server, not this instance: `spec.name` carries
|
||||
// the instance id and a claim named after it would be torn down on the
|
||||
// next server edit — exactly when the data has to survive.
|
||||
|
||||
@@ -54,6 +54,10 @@ export const CreateMcpServerSchema = z.object({
|
||||
// the deadline exists so a wedged call answers instead of hanging, and a
|
||||
// value beyond that is indistinguishable from no deadline at all.
|
||||
toolCallTimeoutSeconds: z.number().int().min(1).max(3600).optional(),
|
||||
// Per-server memory ceiling in MiB. Floor 64 because nothing useful runs
|
||||
// below it; ceiling 16384 so a typo cannot ask for a pod no node can
|
||||
// schedule, which fails as a silently Pending instance rather than an error.
|
||||
memoryLimitMb: z.number().int().min(64).max(16384).optional(),
|
||||
}).refine(
|
||||
(s) => s.volumes.length === 0 || s.replicas <= 1,
|
||||
{
|
||||
@@ -91,6 +95,7 @@ export const UpdateMcpServerSchema = z.object({
|
||||
secretDelivery: SecretDeliverySchema.optional(),
|
||||
entrypoint: z.array(z.string()).nullable().optional(),
|
||||
toolCallTimeoutSeconds: z.number().int().min(1).max(3600).nullable().optional(),
|
||||
memoryLimitMb: z.number().int().min(64).max(16384).nullable().optional(),
|
||||
});
|
||||
|
||||
export type CreateMcpServerInput = z.infer<typeof CreateMcpServerSchema>;
|
||||
|
||||
@@ -70,7 +70,7 @@ function mockOrchestrator(): McpOrchestrator {
|
||||
};
|
||||
}
|
||||
|
||||
function makeServer(overrides: Partial<{ id: string; name: string; replicas: number; dockerImage: string | null; externalUrl: string | null; transport: string; command: unknown; containerPort: number | null; volumes: unknown }> = {}) {
|
||||
function makeServer(overrides: Partial<{ id: string; name: string; replicas: number; dockerImage: string | null; externalUrl: string | null; transport: string; command: unknown; containerPort: number | null; volumes: unknown; memoryLimitMb: number | null }> = {}) {
|
||||
return {
|
||||
id: overrides.id ?? 'srv-1',
|
||||
name: overrides.name ?? 'slack',
|
||||
@@ -83,6 +83,7 @@ function makeServer(overrides: Partial<{ id: string; name: string; replicas: num
|
||||
externalUrl: overrides.externalUrl ?? null,
|
||||
command: overrides.command ?? null,
|
||||
containerPort: overrides.containerPort ?? null,
|
||||
memoryLimitMb: overrides.memoryLimitMb ?? null,
|
||||
replicas: overrides.replicas ?? 1,
|
||||
env: [],
|
||||
version: 1,
|
||||
@@ -191,6 +192,37 @@ describe('InstanceService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('memory limit', () => {
|
||||
// A server that drives a browser (docs-mcp-server scrapes with headless
|
||||
// Chromium) is OOMKilled seconds into its first job at the orchestrator's
|
||||
// 512 MiB default, and an OOMKill says nothing: the pod restarts, the
|
||||
// in-memory job queue is gone, and the instance reads healthy again.
|
||||
it('converts the declared MiB ceiling into the spec, in bytes', async () => {
|
||||
vi.mocked(serverRepo.findById).mockResolvedValue(makeServer({
|
||||
name: 'docs',
|
||||
memoryLimitMb: 2048,
|
||||
}));
|
||||
vi.mocked(instanceRepo.findAll).mockResolvedValue([]);
|
||||
|
||||
await service.reconcile('srv-1');
|
||||
|
||||
const spec = vi.mocked(orchestrator.createContainer).mock.calls[0]![0];
|
||||
expect(spec.memoryLimit).toBe(2048 * 1024 * 1024);
|
||||
});
|
||||
|
||||
it('leaves the spec free of a limit when the server declares none', async () => {
|
||||
// Absent, not zero: the orchestrator falls back to DEFAULT_MEMORY_LIMIT,
|
||||
// so every existing server keeps exactly the limit it has today.
|
||||
vi.mocked(serverRepo.findById).mockResolvedValue(makeServer({}));
|
||||
vi.mocked(instanceRepo.findAll).mockResolvedValue([]);
|
||||
|
||||
await service.reconcile('srv-1');
|
||||
|
||||
const spec = vi.mocked(orchestrator.createContainer).mock.calls[0]![0];
|
||||
expect(spec.memoryLimit).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('reconcile', () => {
|
||||
it('starts instances when below desired replicas', async () => {
|
||||
vi.mocked(serverRepo.findById).mockResolvedValue(makeServer({ replicas: 2 }));
|
||||
|
||||
@@ -45,4 +45,31 @@ describe('McpServerRepository field mapping', () => {
|
||||
} as never);
|
||||
expect(create.mock.calls[0]?.[0].data).toMatchObject({ secretDelivery: 'injector' });
|
||||
});
|
||||
|
||||
it('persists memoryLimitMb on update', async () => {
|
||||
const { spy, update } = prismaSpy();
|
||||
await new McpServerRepository(spy).update('id1', { memoryLimitMb: 2048 });
|
||||
expect(update.mock.calls[0]?.[0].data).toMatchObject({ memoryLimitMb: 2048 });
|
||||
});
|
||||
|
||||
it('persists memoryLimitMb on create, and nulls it when absent', async () => {
|
||||
const { spy, create } = prismaSpy();
|
||||
await new McpServerRepository(spy).create({
|
||||
name: 'x', description: '', transport: 'STDIO', replicas: 1, env: [], volumes: [],
|
||||
memoryLimitMb: 2048,
|
||||
} as never);
|
||||
expect(create.mock.calls[0]?.[0].data).toMatchObject({ memoryLimitMb: 2048 });
|
||||
|
||||
const second = prismaSpy();
|
||||
await new McpServerRepository(second.spy).create({
|
||||
name: 'y', description: '', transport: 'STDIO', replicas: 1, env: [], volumes: [],
|
||||
} as never);
|
||||
expect(second.create.mock.calls[0]?.[0].data).toMatchObject({ memoryLimitMb: null });
|
||||
});
|
||||
|
||||
it('leaves memoryLimitMb untouched on update when not supplied', async () => {
|
||||
const { spy, update } = prismaSpy();
|
||||
await new McpServerRepository(spy).update('id1', { description: 'x' });
|
||||
expect(update.mock.calls[0]?.[0].data ?? {}).not.toHaveProperty('memoryLimitMb');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user