fix(runtime-host): serve revision-consistent Usage snapshots - #4068
fix(runtime-host): serve revision-consistent Usage snapshots#4068Sun-GLiang wants to merge 15 commits into
Conversation
…shot-consistency # Conflicts: # packages/runtime-host/src/protocol/index.ts
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for moving Usage reads onto one coherent revision; the Storage/Host/Desktop ownership is much clearer now. I found one remaining lifecycle boundary where a valid reader can lose its revision mid-pagination. This is a suggestion from an outside review, so please feel free to push back if the supported concurrency or latency envelope is intentionally narrower.
AI-assisted review disclosure: Codex ran independent authority and production/test analysis lanes; Astro-Han is the contributor of record for this review.
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
…shot-consistency # Conflicts: # apps/desktop/src/main/runtime-host-client.ts # packages/runtime-host/src/protocol/index.ts
…shot-consistency # Conflicts: # docs/windows-test-inventory.md # packages/runtime-host/src/protocol/index.ts # packages/runtime-host/src/server/operation-dispatcher.ts
Astro-Han
left a comment
There was a problem hiding this comment.
The lease rework closes last round's point properly: reserve() has no eviction path, capacity is taken before capture (the barrier test proves the fifth start is refused before any title read), idle renews under a hard cap, ownership is checked on read and release, and release rides the existing releaseConnection seam with a real five-client UDS test for disconnect. Capture is one BEGIN IMMEDIATE on the shared handle, so the consistency claim holds in-process, and paging no longer re-runs the unbounded model-call read per page. Build, 55 host and storage tests, 15 desktop tests, lint, format, typecheck and the architecture check are green locally on bdf0faf5; merge-tree against main is clean.
One thing to fix before merge. Your reply says the fifth start "returns the typed revision_changed result"; the code returns operation_conflict, and loadUsageSnapshot only retries on revision_changed, so the error reaches the Usage page as a hard failure. Two supported paths get there: a single Desktop connection switching ranges quickly holds all four slots until its own finally runs, so a second window or another client fails outright; and a best-effort release that times out on a remote Host leaves the lease until idle expiry, so four of those is five minutes of failure for everyone. Both are recoverable, but #4058 item four asks for a bounded retry of the whole load. Smallest fix: a per-connection cap in reserve() (one is enough for Desktop) and operation_conflict inside the existing MAX_USAGE_SNAPSHOT_ATTEMPTS loop with a short backoff.
Things this PR makes redundant and should take with it: after loadAllLogs goes, usage.query kind: 'logs' has no production consumer, and the Desktop usage:logs and usage:buckets handlers were never exposed by preload on main either. Deleting those two handlers, loadAllBuckets, the logs/buckets protocol variants and the coordinator's usageLogPage family, then folding the old and new page builders, is a few hundred lines of net deletion on an epoch this PR already bumps. Keep kind: 'summary', Session Inspector uses it.
Smaller: the 50,000 activity cap lives in both usage-snapshot-cache.ts and runtime-host-client.ts, and the Desktop copy turns a legitimately larger Host page into invalidProjection; carry it in snapshot_started or the protocol. retain() on the cache is test-only, production goes reserve then finalize. The inner transaction('read') inside the outer write transaction is a pass-through at depth one, and the third acquireOperationalStateDatabase can come from the repos' existing lease. The started.kind !== 'snapshot_started' branch sits outside the try and is unreachable after assertUsageQueryOutputForInput, and the test that pins it can go with it. The body still says epoch 79; the code is 95, which #4386, #4308, #4439, #4500 and #4508 also claim, so re-check at merge.
Evidence boundary: static read of bdf0faf5 against main 92fa5281; runtime-host, storage and desktop usage suites run locally; Playwright not run, and the two-window and rapid-range scenarios are traced, not exercised.
AI-assisted review: drafted with Maka; I verified the capacity error path, the Desktop retry condition and the preload exposure myself.
简体中文
租约改造把上轮的点关干净了:不驱逐、先占容量再捕获、idle 续期加硬上限、归属校验、断连回收走现有接缝,一次 BEGIN IMMEDIATE 保证进程内一致性。本地验证全绿。合并前要修一处:第 5 个 start 实际返回 operation_conflict 而非你回复里说的 revision_changed,Desktop 只对 revision_changed 重试,所以用户看到的是硬失败;单连接快速切 range 就能占满四个 slot 饿死其他客户端。最小修法:reserve() 加每连接上限,并把 operation_conflict 纳入现有重试循环。本 PR 让 usage.query 的 logs 变体和 Desktop 两个从未经 preload 暴露的 IPC handler 变成死代码,建议同 PR 删掉。其余为小项:50,000 上限双权威、test-only 的 retain()、无效的事务嵌套、不可达的 kind 分支、正文 epoch 79 应为 95。
…shot-consistency # Conflicts: # packages/runtime-host/src/protocol/index.ts
Resolve the Runtime Host protocol epoch conflict by placing the Usage snapshot wire contract at epoch 100 after main's epochs 97-99. Fence the WorkHub layout E2E first send on the shared send-readiness signal; an unfenced Enter could be silently dropped while submission admission was still initializing. Generated-by: Codex
The prior run stopped when Node 24 could not deserialize its test-runner child payload. The affected release-contract file is unchanged and passes 10/10 isolated repetitions locally.\n\nGenerated-by: Codex
Bound snapshot leases per connection while preserving one replacement load, retry capacity conflicts as whole Desktop loads, and enforce the shared activity ceiling at the protocol boundary.\n\nGenerated-by: Codex
|
Review disposition for 3415280: Implemented:
I did not apply the remaining suggestions verbatim:
Fresh local verification: full build; Runtime Host 1,618 passed / 12 skipped; Desktop 1,983 passed; Storage 1,086 passed / 8 skipped; typecheck, lint, format, protocol epoch guard, and independent review all passed. |
Resolve the concurrent Runtime Host wire changes at compatibility epoch 101.\n\nGenerated-by: Codex
|
Follow-up after the latest main sync: main advanced while the review fixes were being pushed and introduced a separate wire change at epoch 100. The merge conflict is resolved in f599d71 by preserving that history and moving this PR's Usage snapshot wire change to epoch 101; the predecessor-handshake test now rejects epoch-100 peers. CI run 33719193127 passed in full: https://github.com/apache/maka/actions/runs/33719193127 |
Summary
finallypath, preserve the latestmainsession-title hydration with bounded concurrency, and raise the merged Runtime Host compatibility epoch to 101Fixes #4058
Verification
npm run buildnpm run typechecknpm run format:checknpm run lintgit diff --checkAI use
Select exactly one:
Tool(s) and scope: Codex implemented and tested the review remediation, Desktop/Runtime Host lease lifecycle, capacity reservation, and merge-conflict resolution. Sun-GLiang is the human contributor of record.
Checklist
Does this PR entail a change in behavior?