-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(serve): observe daemon and child memory against real denominators #8423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
0e2e1f7
feat(serve): observe daemon memory pressure against a real denominator
doudouOUC 29a7d1d
feat(serve): report aggregate ACP child RSS, not just the primary's (…
doudouOUC e6793a3
test(serve): close the under-determined assertions review probed
doudouOUC ead69a3
refactor(serve): model a per-child heap partition of the daemon budge…
doudouOUC 8b3e0ee
fix(serve): repair the child-heap assertion and the reservation leak
doudouOUC e8aafa6
fix(serve): report no child-heap partition under `off`
doudouOUC 9ad06a8
Merge branch 'main' into agent/daemon-memory-observe
wenshao 6872d59
fix(serve): never model a child heap ceiling below the documented min…
doudouOUC a17ce45
Merge remote-tracking branch 'origin/agent/daemon-memory-observe' int…
doudouOUC cf27d75
Merge branch 'main' into agent/daemon-memory-observe
wenshao a909ff5
Merge remote-tracking branch 'origin/agent/daemon-memory-observe' int…
doudouOUC File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] R3-14: This paragraph is contradicted by what this same PR ships. 'nothing in it can produce a stale reading, since the figures are sampled synchronously as the response is built' — but runtime.memory.children reads cached poll results up to STALE_CHILD_RESOURCE_MS (~30s) old, which is exactly why the diff adds ageMs and oldestReadingAgeMs. The closing promise 'It arrives with the polled measurements that can actually go stale' is broken by this PR itself: those polled measurements landed here, and the issue code did not. — Failure scenario: this design doc is the stated reference for the deferred enforce mode; a future implementer records two false beliefs — that every shipped runtime.memory figure is sampled per status request, and that the stale-observation code still waits on some future polled measurement — when the shipped children block is already that measurement. Suggested fix: rewrite to match what shipped — pressure is sampled synchronously per request, but the aggregate child RSS reads cached poll results persisting up to ~30s (disclosed via oldestReadingAgeMs); the second issue code remains deferred, with the real reason stated.
中文说明
该段落与本 PR 自身发布的内容矛盾。「nothing in it can produce a stale reading, since the figures are sampled synchronously as the response is built」——但 runtime.memory.children 读取的是最长可达 STALE_CHILD_RESOURCE_MS(~30s)的缓存轮询结果,这正是 diff 新增 ageMs 与 oldestReadingAgeMs 的原因。结尾的承诺「It arrives with the polled measurements that can actually go stale」被本 PR 自己打破:那些会过期的轮询测量已经落地,而 issue 代码没有。失效场景:该设计文档是延迟的 enforce 模式明示的参考;未来实现者会记下两个错误认知——以为每个已发布的 runtime.memory 数值都是每次 status 请求现采的、以为 stale-observation 代码仍在等待某种未来的轮询测量——而已发布的 children 块就是那个测量。建议:改写为与实际发布一致——pressure 每次请求现采,但 children 汇总读取最长保留 ~30s 的缓存轮询结果(经 oldestReadingAgeMs 披露);第二个 issue 代码仍延迟,并写明真实原因。
— qwen3.8-max via Qwen Code /review (v0.21.5)