mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-10 18:56:53 +00:00
perf(stacks): remove repeated self-identity discovery from statuses hot path (#1811)
* perf(stacks): hoist self-stack identity resolution out of statuses enrichment
GET /api/stacks/statuses resolved the self-stack identity once per stack on
every request, including cache hits. The fallback path in isSelfStack lists
every container on the node, so a cache hit still paid N container-list
calls. Measured on a Docker-shaped local container with 6 stacks: cache hits
took 161-168ms with dockerMs=null; the enrichment alone was ~160ms.
Resolve the identity once per request (boot-cached compose project name plus
a single container-labels read) and compare stack names against it in the
response loop. isSelf semantics are unchanged: project-name match, then
labels project match, then working-dir basename match, with the same
fail-safe false on identity failure. Also expose an enrichmentMs subspan in
the developer-mode timing line so cache-hit cost stays decomposable.
Measured after the change in the same environment: cache hits 20-34ms
(enrichment ~20-33ms, a single listContainers call), computed 44-59ms.
Cache-hit latency improved roughly 5-8x; per-stack Docker identity discovery
is eliminated (N calls down to 1).
* test(stacks): cover the empty-stacks statuses path
The statuses handler skips self-stack identity resolution entirely when a
node has no stacks. Pin that branch: an empty response still returns 200
with {} and never resolves identity, so a future refactor cannot silently
reintroduce a node-wide container-list call on empty-node polls.
This commit is contained in:
@@ -20,6 +20,7 @@ import { setupTestDb, cleanupTestDb, TEST_USERNAME, TEST_PASSWORD } from './help
|
||||
import { installArcstatsFsMock, arcstatsBody, DEFAULT_ARC_PATH, type ArcstatsFsMock } from './helpers/arcstatsFsMock';
|
||||
import { GitSourceService } from '../services/GitSourceService';
|
||||
import type { PublicGitSource } from '../services/GitSourceService';
|
||||
import * as selfStackGuard from '../helpers/selfStackGuard';
|
||||
|
||||
// ── Hoisted mocks (must come before importing the app) ─────────────────
|
||||
|
||||
@@ -284,6 +285,63 @@ describe('GET /api/stacks/statuses caching', () => {
|
||||
listSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('resolves self-stack identity once per request, cache hits included, and labels each stack from it', async () => {
|
||||
mockGetStacks.mockResolvedValue(['sencho.yml', 'web.yml']);
|
||||
mockGetBulkStackStatuses.mockResolvedValue({
|
||||
sencho: { status: 'running' },
|
||||
web: { status: 'running' },
|
||||
});
|
||||
const identitySpy = vi
|
||||
.spyOn(selfStackGuard, 'resolveSelfStackIdentity')
|
||||
.mockResolvedValue({ projectName: 'sencho', labels: null });
|
||||
|
||||
const first = await request(app).get('/api/stacks/statuses').set('Cookie', authCookie);
|
||||
const second = await request(app).get('/api/stacks/statuses').set('Cookie', authCookie);
|
||||
|
||||
// Hoisted: one resolution per request (including the cache hit) serves
|
||||
// every stack in the response.
|
||||
expect(identitySpy).toHaveBeenCalledTimes(2);
|
||||
for (const res of [first, second]) {
|
||||
expect(res.body['sencho.yml'].isSelf).toBe(true);
|
||||
expect(res.body['web.yml'].isSelf).toBe(false);
|
||||
}
|
||||
|
||||
identitySpy.mockRestore();
|
||||
});
|
||||
|
||||
it('skips identity resolution entirely when the node has no stacks', async () => {
|
||||
mockGetStacks.mockResolvedValue([]);
|
||||
mockGetBulkStackStatuses.mockResolvedValue({});
|
||||
const identitySpy = vi.spyOn(selfStackGuard, 'resolveSelfStackIdentity');
|
||||
|
||||
const res = await request(app).get('/api/stacks/statuses').set('Cookie', authCookie);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({});
|
||||
expect(identitySpy).not.toHaveBeenCalled();
|
||||
|
||||
identitySpy.mockRestore();
|
||||
});
|
||||
|
||||
it('serves 200 with isSelf false everywhere when identity resolution degrades', async () => {
|
||||
mockGetStacks.mockResolvedValue(['web.yml', 'db.yml']);
|
||||
mockGetBulkStackStatuses.mockResolvedValue({
|
||||
web: { status: 'running' },
|
||||
db: { status: 'running' },
|
||||
});
|
||||
const identitySpy = vi
|
||||
.spyOn(selfStackGuard, 'resolveSelfStackIdentity')
|
||||
.mockResolvedValue({ projectName: null, labels: null });
|
||||
|
||||
const res = await request(app).get('/api/stacks/statuses').set('Cookie', authCookie);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body['web.yml'].isSelf).toBe(false);
|
||||
expect(res.body['db.yml'].isSelf).toBe(false);
|
||||
|
||||
identitySpy.mockRestore();
|
||||
});
|
||||
|
||||
it('falls back to local labels (200, not 500) when the git-source lookup throws', async () => {
|
||||
mockGetStacks.mockResolvedValue(['web.yml']);
|
||||
mockGetBulkStackStatuses.mockResolvedValue({ web: { status: 'running' } });
|
||||
|
||||
@@ -103,6 +103,10 @@ describe('[Stacks:debug] GET /api/stacks/statuses', () => {
|
||||
expect(second).toMatch(/cacheOutcome=hit/);
|
||||
// No docker call on a cache hit, so the subspan is null rather than 0.
|
||||
expect(second).toMatch(/dockerMs=null/);
|
||||
// Enrichment runs on every request, cache hits included, so its subspan
|
||||
// is a number on both legs.
|
||||
expect(first).toMatch(/enrichmentMs=\d+/);
|
||||
expect(second).toMatch(/enrichmentMs=\d+/);
|
||||
// The compute ran the fetcher exactly once across both requests.
|
||||
expect(dockerCalls).toBe(1);
|
||||
});
|
||||
|
||||
@@ -3,7 +3,9 @@ import SelfIdentityService from '../services/SelfIdentityService';
|
||||
import DockerController from '../services/DockerController';
|
||||
import {
|
||||
isSelfStack,
|
||||
isSelfStackByIdentity,
|
||||
getSelfStackProjectName,
|
||||
resolveSelfStackIdentity,
|
||||
SELF_STACK_PROTECTED_CODE,
|
||||
SELF_STACK_PROTECTED_MESSAGE,
|
||||
selfStackProtectedBulkResult,
|
||||
@@ -93,6 +95,96 @@ describe('getSelfStackProjectName', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveSelfStackIdentity + isSelfStackByIdentity', () => {
|
||||
it('matches the stack whose name equals the resolved project name', async () => {
|
||||
stubComposeProject('sencho');
|
||||
const identity = await resolveSelfStackIdentity();
|
||||
expect(isSelfStackByIdentity(identity, 'sencho')).toBe(true);
|
||||
expect(isSelfStackByIdentity(identity, 'web')).toBe(false);
|
||||
});
|
||||
|
||||
it('matches by the container compose project label when the project name is unknown', async () => {
|
||||
const runtimeId = 'c'.repeat(64);
|
||||
process.env.HOSTNAME = runtimeId.slice(0, 12);
|
||||
stubComposeProject(null);
|
||||
vi.spyOn(DockerController, 'getInstance').mockReturnValue({
|
||||
getDocker: () => ({
|
||||
listContainers: vi.fn().mockResolvedValue([
|
||||
{
|
||||
Id: runtimeId,
|
||||
Labels: { 'com.docker.compose.project': 'renamed-project' },
|
||||
},
|
||||
]),
|
||||
}),
|
||||
} as unknown as ReturnType<typeof DockerController.getInstance>);
|
||||
|
||||
const identity = await resolveSelfStackIdentity();
|
||||
expect(isSelfStackByIdentity(identity, 'renamed-project')).toBe(true);
|
||||
expect(isSelfStackByIdentity(identity, 'web')).toBe(false);
|
||||
});
|
||||
|
||||
it('matches by the working directory basename inside the compose dir', async () => {
|
||||
const runtimeId = 'd'.repeat(64);
|
||||
process.env.HOSTNAME = runtimeId.slice(0, 12);
|
||||
stubComposeProject(null);
|
||||
vi.spyOn(DockerController, 'getInstance').mockReturnValue({
|
||||
getDocker: () => ({
|
||||
listContainers: vi.fn().mockResolvedValue([
|
||||
{
|
||||
Id: runtimeId,
|
||||
Labels: { 'com.docker.compose.project.working_dir': '/app/compose/sencho' },
|
||||
},
|
||||
]),
|
||||
}),
|
||||
} as unknown as ReturnType<typeof DockerController.getInstance>);
|
||||
|
||||
const identity = await resolveSelfStackIdentity();
|
||||
expect(isSelfStackByIdentity(identity, 'sencho', '/app/compose')).toBe(true);
|
||||
});
|
||||
|
||||
it('does not match a working directory outside the compose dir', async () => {
|
||||
const runtimeId = 'g'.repeat(64);
|
||||
process.env.HOSTNAME = runtimeId.slice(0, 12);
|
||||
stubComposeProject(null);
|
||||
vi.spyOn(DockerController, 'getInstance').mockReturnValue({
|
||||
getDocker: () => ({
|
||||
listContainers: vi.fn().mockResolvedValue([
|
||||
{
|
||||
Id: runtimeId,
|
||||
Labels: { 'com.docker.compose.project.working_dir': '/srv/other/sencho' },
|
||||
},
|
||||
]),
|
||||
}),
|
||||
} as unknown as ReturnType<typeof DockerController.getInstance>);
|
||||
|
||||
const identity = await resolveSelfStackIdentity();
|
||||
expect(isSelfStackByIdentity(identity, 'sencho', '/app/compose')).toBe(false);
|
||||
});
|
||||
|
||||
it('is false when no identity source is available', async () => {
|
||||
stubComposeProject(null);
|
||||
const identity = await resolveSelfStackIdentity();
|
||||
expect(isSelfStackByIdentity(identity, 'sencho')).toBe(false);
|
||||
});
|
||||
|
||||
it('resolves identity with at most one container list read, shared by all stacks', async () => {
|
||||
const runtimeId = 'e'.repeat(64);
|
||||
process.env.HOSTNAME = runtimeId.slice(0, 12);
|
||||
stubComposeProject('sencho');
|
||||
const listContainers = vi.fn().mockResolvedValue([]);
|
||||
vi.spyOn(DockerController, 'getInstance').mockReturnValue({
|
||||
getDocker: () => ({ listContainers }),
|
||||
} as unknown as ReturnType<typeof DockerController.getInstance>);
|
||||
|
||||
const identity = await resolveSelfStackIdentity();
|
||||
// Reuse the resolved identity across every stack in the request.
|
||||
for (const name of ['sencho', 'web', 'db', 'cache']) {
|
||||
isSelfStackByIdentity(identity, name, '/app/compose');
|
||||
}
|
||||
expect(listContainers).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('selfStackProtectedBulkResult', () => {
|
||||
it('returns a per-stack bulk failure with the protected code', () => {
|
||||
const result = selfStackProtectedBulkResult('sencho');
|
||||
|
||||
@@ -106,6 +106,45 @@ export async function isSelfStack(stackName: string, composeDir?: string): Promi
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Identity sources for the self-stack check, resolved once and reused
|
||||
* across every stack in a request (the /statuses handler resolves it once
|
||||
* per request). The compose project name is a boot-cached property lookup;
|
||||
* the container labels fallback (which lists every container on the node)
|
||||
* is a single read shared by all stacks. Either source degrades
|
||||
* independently to null, so a failure in one never discards the other.
|
||||
*/
|
||||
export interface SelfStackIdentity {
|
||||
projectName: string | null;
|
||||
labels: Record<string, string> | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves both identity sources. Each failure degrades only its own
|
||||
* source to null, matching the old per-stack behavior where a labels
|
||||
* failure never discarded the resolved project name.
|
||||
*/
|
||||
export async function resolveSelfStackIdentity(): Promise<SelfStackIdentity> {
|
||||
const projectName = await getSelfStackProjectName().catch((error) => {
|
||||
console.error('Failed to resolve self-stack project name; self-stack check degraded:', error);
|
||||
return null;
|
||||
});
|
||||
const labels = await getRunningContainerLabels().catch((error) => {
|
||||
console.error('Failed to resolve self-stack container labels; self-stack check degraded:', error);
|
||||
return null;
|
||||
});
|
||||
return { projectName, labels };
|
||||
}
|
||||
|
||||
/** isSelf semantics identical to isSelfStack() when the identity resolves successfully. */
|
||||
export function isSelfStackByIdentity(identity: SelfStackIdentity, stackName: string, composeDir?: string): boolean {
|
||||
if (identity.projectName === stackName) return true;
|
||||
const labels = identity.labels;
|
||||
if (!labels) return false;
|
||||
if (labels['com.docker.compose.project'] === stackName) return true;
|
||||
return workingDirMatchesStack(labels['com.docker.compose.project.working_dir'], stackName, composeDir);
|
||||
}
|
||||
|
||||
export interface SelfStackProtectedResult {
|
||||
stackName: string;
|
||||
ok: false;
|
||||
|
||||
@@ -69,7 +69,13 @@ import { parseComposeSelection, defaultEnvPath } from '../helpers/gitSourceSelec
|
||||
import { resolveStackEnvSources, discoverStackLocalEnvFiles } from '../helpers/envFileResolution';
|
||||
import { STACK_STATUSES_CACHE_TTL_MS } from '../helpers/constants';
|
||||
import { getTerminalWs, DEPLOY_SESSION_HEADER } from '../websocket/generic';
|
||||
import { isSelfStack, refuseIfSelfStack, selfStackProtectedBulkResult } from '../helpers/selfStackGuard';
|
||||
import {
|
||||
isSelfStack,
|
||||
isSelfStackByIdentity,
|
||||
refuseIfSelfStack,
|
||||
resolveSelfStackIdentity,
|
||||
selfStackProtectedBulkResult,
|
||||
} from '../helpers/selfStackGuard';
|
||||
import { getActiveCapabilities, STACK_DOWN_REMOVE_VOLUMES_CAPABILITY, SERVICE_SCOPED_UPDATE_CAPABILITY } from '../services/CapabilityRegistry';
|
||||
import { ServiceUpdateRecoveryService } from '../services/ServiceUpdateRecoveryService';
|
||||
import { classifyStackApiPath } from '../helpers/stackRouteAuth';
|
||||
@@ -335,6 +341,7 @@ stacksRouter.get('/statuses', async (req: Request, res: Response) => {
|
||||
let outcome: 'ok' | 'error' = 'ok';
|
||||
let cacheOutcome: CacheFetchOutcome | null = null;
|
||||
let dockerMs: number | null = null;
|
||||
let enrichmentMs: number | null = null;
|
||||
let count = 0;
|
||||
try {
|
||||
const { value: result, outcome: fetchOutcome } = await CacheService.getInstance().getOrFetchWithMeta(
|
||||
@@ -357,6 +364,7 @@ stacksRouter.get('/statuses', async (req: Request, res: Response) => {
|
||||
);
|
||||
cacheOutcome = fetchOutcome;
|
||||
count = Object.keys(result).length;
|
||||
const enrichmentStartedAt = Date.now();
|
||||
// Git-source labels are computed live, outside the cache, so linking or
|
||||
// unlinking a stack's Git source is reflected immediately. The Docker
|
||||
// status portion keeps its short TTL; only the cheap source label is fresh.
|
||||
@@ -368,17 +376,20 @@ stacksRouter.get('/statuses', async (req: Request, res: Response) => {
|
||||
} catch (sourceError) {
|
||||
console.error('Failed to load git sources for status labels; defaulting to local:', sourceError);
|
||||
}
|
||||
// Self-stack identity is resolved once per request instead of once per
|
||||
// stack, so cache hits no longer pay N container-list calls.
|
||||
const selfIdentity = count > 0 ? await resolveSelfStackIdentity() : { projectName: null, labels: null };
|
||||
const withSource: Record<string, BulkStackInfo & { source: 'local' | 'git' }> = {};
|
||||
const composeDir = FileSystemService.getInstance(req.nodeId).getBaseDir();
|
||||
for (const [stack, info] of Object.entries(result)) {
|
||||
const name = stack.replace(/\.(yml|yaml)$/, '');
|
||||
const isSelf = await isSelfStack(name, composeDir);
|
||||
withSource[stack] = {
|
||||
...info,
|
||||
source: gitStackNames.has(name) ? 'git' : 'local',
|
||||
isSelf,
|
||||
isSelf: isSelfStackByIdentity(selfIdentity, name, composeDir),
|
||||
};
|
||||
}
|
||||
enrichmentMs = Date.now() - enrichmentStartedAt;
|
||||
res.json(withSource);
|
||||
} catch (error) {
|
||||
outcome = 'error';
|
||||
@@ -391,6 +402,7 @@ stacksRouter.get('/statuses', async (req: Request, res: Response) => {
|
||||
cacheOutcome,
|
||||
count,
|
||||
dockerMs,
|
||||
enrichmentMs,
|
||||
elapsedMs: Date.now() - startedAt,
|
||||
outcome,
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user