Skip to content

Commit 61cf0e0

Browse files
os-zhuangclaude
andcommitted
perf(rest): cache hostname→env resolution (P1-4); document cluster pub/sub durability (P1-5)
P1-4: resolveByHostname() ran on every unscoped request (a control-plane DB lookup in the hot path). RestServer.resolveHostnameCached() now caches hostname→environmentId for 30s across all 3 call sites, including negative results so unknown hosts don't hammer the registry; registry errors aren't cached so a transient blip self-heals. +3 tests. P1-5: verified the Redis pub/sub fire-and-forget is by design (at-most-once) and already implied in code. Recorded the durability contract in pubsub.ts: metadata.changed is a cache-invalidation hint only — the durable source of truth is the transactional sys_metadata (+ sys_metadata_history) write, so a missed event self-heals on reload and never loses data. No delivery-semantics change; risk accepted + documented. docs/launch-readiness.md P1-4/P1-5 updated (Verify ✅, Sign-off left for the team). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 93f97b2 commit 61cf0e0

5 files changed

Lines changed: 106 additions & 5 deletions

File tree

.changeset/p1-control-plane.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/rest': patch
3+
'@objectstack/service-cluster-redis': patch
4+
---
5+
6+
perf(rest): cache hostname→environment resolution; document cluster pub/sub durability (P1-4, P1-5)
7+
8+
- **rest (P1-4):** `resolveByHostname()` ran on every unscoped request — a
9+
control-plane lookup (typically a DB query) in the hot path. `RestServer` now
10+
caches `hostname → environmentId` in-memory with a 30s TTL across all three
11+
resolution sites, caching negative results too so unknown hosts don't hammer the
12+
registry. Registry errors are not cached, so a transient blip self-heals.
13+
- **service-cluster-redis (P1-5):** recorded the durability contract for
14+
`metadata.changed` in `pubsub.ts`. Redis pub/sub is at-most-once **by design**;
15+
the event is a cache-invalidation hint only — the durable source of truth is the
16+
transactional `sys_metadata` (+ `sys_metadata_history`) write, so a missed event
17+
causes a stale cache until the next reload, never data loss. No code change to
18+
the delivery semantics; risk accepted and documented.

docs/launch-readiness.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ fix or acceptance.**
144144
- **Risk:** `resolveByHostname()` runs on every unscoped request → control-plane
145145
latency spike under load; silent fallback to default project masks it.
146146
- **Action:** Add an in-memory TTL cache (~30s) for `hostname → environmentId`.
147-
- **Owner:** _______ · Verify · Sign-off ☐ · Notes: _______
147+
- **Owner:** _______ · Verify ✅ (confirmed real @ `main`) · Sign-off ☐ · Notes: Fixed — `RestServer.resolveHostnameCached()` caches hostname→env (positive **and** negative) for 30s across all 3 call sites; +3 tests. Awaiting human sign-off.
148148

149149
### P1-5 — Cluster pub/sub is fire-and-forget (metadata-changed)
150150
- **Area:** `service-cluster-redis``src/pubsub.ts:~75–90`
@@ -153,7 +153,14 @@ fix or acceptance.**
153153
- **Action:** Acceptable for non-critical events if documented; ensure schema
154154
mutations re-sync on error boundaries (history exists in `sys_metadata_history`).
155155
Record the durability contract.
156-
- **Owner:** _______ · Verify ☐ · Sign-off ☐ · Notes: _______
156+
- **Durability contract (recorded):** Redis pub/sub is **at-most-once** by design
157+
(already noted in `pubsub.ts`). `metadata.changed` is a **cache-invalidation hint
158+
only** — the durable source of truth is the transactional write to `sys_metadata`
159+
(+ `sys_metadata_history`). A node that misses the event serves its cached schema
160+
until the next reload and **loses no data** (self-heals on reload/restart against
161+
the DB). Documented in `pubsub.ts publish()`. **Accept** for v1: no exactly-once
162+
state may flow through this channel — durable state uses an outbox.
163+
- **Owner:** _______ · Verify ✅ (by design — contract recorded) · Sign-off ☐ · Notes: No code fix needed; risk **accepted** with rationale + code comment. Awaiting human sign-off.
157164

158165
---
159166

packages/rest/src/rest-server.ts

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,15 @@ export class RestServer {
437437
private routeManager: RouteManager;
438438
private kernelManager?: RestKernelManager;
439439
private envRegistry?: RestEnvRegistry;
440+
/**
441+
* Short-TTL cache for `hostname → environmentId` (P1-4). `resolveByHostname`
442+
* is a control-plane lookup (typically a DB query) that otherwise runs on
443+
* *every* unscoped request; caching it — including negative results, so
444+
* unknown hosts don't hammer the registry — removes that per-request cost.
445+
* The TTL is short so a newly-bound hostname becomes routable quickly.
446+
*/
447+
private readonly hostnameCache = new Map<string, { value: { environmentId: string } | null; expiresAt: number }>();
448+
private readonly hostnameCacheTtlMs = 30_000;
440449
private defaultEnvironmentIdProvider?: () => string | undefined;
441450
private authServiceProvider?: (environmentId?: string) => Promise<any | undefined>;
442451
private objectQLProvider?: (environmentId?: string) => Promise<any | undefined>;
@@ -501,13 +510,29 @@ export class RestServer {
501510
* (and any other client) speak a single, uniform URL family without
502511
* duplicating route logic for the platform surface.
503512
*/
513+
/**
514+
* Cached wrapper around `envRegistry.resolveByHostname` (P1-4). Returns the
515+
* cached result while fresh; on a miss it queries the registry and caches the
516+
* outcome (positive *and* negative) for {@link hostnameCacheTtlMs}. Registry
517+
* errors are not cached so a transient control-plane blip self-heals on the
518+
* next request.
519+
*/
520+
private async resolveHostnameCached(host: string): Promise<{ environmentId: string } | null | undefined> {
521+
const now = Date.now();
522+
const hit = this.hostnameCache.get(host);
523+
if (hit && hit.expiresAt > now) return hit.value;
524+
const result = (await this.envRegistry!.resolveByHostname(host)) ?? null;
525+
this.hostnameCache.set(host, { value: result, expiresAt: now + this.hostnameCacheTtlMs });
526+
return result;
527+
}
528+
504529
private async resolveProtocol(environmentId?: string, req?: any): Promise<ObjectStackProtocol> {
505530
if (environmentId === 'platform') return this.protocol;
506531
if (!environmentId && req && this.envRegistry && this.kernelManager) {
507532
const host = this.extractHostname(req);
508533
if (host) {
509534
try {
510-
const result = await this.envRegistry.resolveByHostname(host);
535+
const result = await this.resolveHostnameCached(host);
511536
if (result?.environmentId) environmentId = result.environmentId;
512537
} catch {
513538
// fall through to next strategy
@@ -567,7 +592,7 @@ export class RestServer {
567592
const host = this.extractHostname(req);
568593
if (host) {
569594
try {
570-
const result = await this.envRegistry.resolveByHostname(host);
595+
const result = await this.resolveHostnameCached(host);
571596
if (result?.environmentId) environmentId = result.environmentId;
572597
} catch { /* fall through */ }
573598
}
@@ -644,7 +669,7 @@ export class RestServer {
644669
const host = this.extractHostname(req);
645670
if (host) {
646671
try {
647-
const result = await this.envRegistry.resolveByHostname(host);
672+
const result = await this.resolveHostnameCached(host);
648673
if (result?.environmentId) environmentId = result.environmentId;
649674
} catch { /* fall through */ }
650675
}

packages/rest/src/rest.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,6 +1324,43 @@ describe('RestServer.resolveProtocol', () => {
13241324
expect(f.kernelManager.getOrCreate).toHaveBeenCalledWith('proj_a');
13251325
});
13261326

1327+
it('caches hostname→env resolution within the TTL and refreshes after it (P1-4)', async () => {
1328+
const projectKernel = makeKernel('proj_a');
1329+
const resolveByHostname = vi.fn().mockResolvedValue({ environmentId: 'proj_a' });
1330+
const f = makeFixture({
1331+
envRegistry: { resolveByHostname, resolveById: vi.fn() },
1332+
kernels: { proj_a: projectKernel },
1333+
});
1334+
let now = 1_000_000;
1335+
const spy = vi.spyOn(Date, 'now').mockImplementation(() => now);
1336+
try {
1337+
const req = { headers: { host: 'a.example.com' } };
1338+
await (f.rest as any).resolveProtocol(undefined, req);
1339+
await (f.rest as any).resolveProtocol(undefined, req);
1340+
await (f.rest as any).resolveProtocol(undefined, req);
1341+
expect(resolveByHostname).toHaveBeenCalledTimes(1); // 2nd/3rd served from cache
1342+
1343+
now += 31_000; // past the 30s TTL
1344+
await (f.rest as any).resolveProtocol(undefined, req);
1345+
expect(resolveByHostname).toHaveBeenCalledTimes(2); // refreshed
1346+
} finally {
1347+
spy.mockRestore();
1348+
}
1349+
});
1350+
1351+
it('caches a negative result so unknown hosts do not hammer the registry (P1-4)', async () => {
1352+
const resolveByHostname = vi.fn().mockResolvedValue(null);
1353+
const f = makeFixture({
1354+
envRegistry: { resolveByHostname, resolveById: vi.fn() },
1355+
defaultProvider: () => 'proj_local',
1356+
kernels: { proj_local: makeKernel('proj_local') },
1357+
});
1358+
const req = { headers: { host: 'unknown.example.com' } };
1359+
await (f.rest as any).resolveProtocol(undefined, req);
1360+
await (f.rest as any).resolveProtocol(undefined, req);
1361+
expect(resolveByHostname).toHaveBeenCalledTimes(1); // negative result cached
1362+
});
1363+
13271364
it('routes via X-Environment-Id header when hostname resolution fails', async () => {
13281365
const projectKernel = makeKernel('proj_b');
13291366
const resolveById = vi.fn().mockResolvedValue({ /* truthy driver */ });

packages/services/service-cluster-redis/src/pubsub.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,20 @@ export class RedisPubSub implements IPubSub {
7575
});
7676
}
7777

78+
/**
79+
* Durability contract (P1-5): this awaits the Redis `PUBLISH` command (so the
80+
* message left this node), but Redis pub/sub is **at-most-once** — there is no
81+
* delivery guarantee to subscribers and no replay for a node that was down or
82+
* slow at publish time. This is acceptable **only** for events that are pure
83+
* cache-invalidation hints, never the source of truth.
84+
*
85+
* In particular `metadata.changed` is such a hint: the durable record of every
86+
* metadata mutation is the transactional write to `sys_metadata`
87+
* (+ `sys_metadata_history`). A subscriber that misses the event keeps serving
88+
* its cached schema until its next reload and **loses no data** — it self-heals
89+
* on restart / reload against the DB. Do not route any state that must be
90+
* delivered exactly-once through this channel; use a durable outbox instead.
91+
*/
7892
async publish<T = unknown>(
7993
channel: string,
8094
payload: T,

0 commit comments

Comments
 (0)