fix(collector): fail closed on shadow memory pressure - #54
Conversation
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe shadow gate now records and validates cgroup ChangesShadow memory stability gate
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Systemd
participant ShadowGate
participant CgroupMemoryEvents
participant GatePolicy
participant GateJson
Systemd->>ShadowGate: Start shadow and provide MainPID
ShadowGate->>CgroupMemoryEvents: Read baseline counters
CgroupMemoryEvents-->>ShadowGate: Return memory.events
ShadowGate->>CgroupMemoryEvents: Check stable counters during monitoring
CgroupMemoryEvents-->>ShadowGate: Return unchanged counters
ShadowGate->>GateJson: Write start and end snapshots
GateJson->>GatePolicy: Validate memory_events and stability check
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51a4f21878
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
51a4f21 to
c59f33d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
deployment/aliyun/test-polymarket-raw-ops-control-plane.sh (2)
1275-1277: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the production unit’s
MemoryMaxtoo.The test verifies the full shadow envelope but only
MemoryHighfor the production collector.Proposed test addition
grep -Fxq 'MemoryHigh=576M' "$SCRIPT_DIR/polymarket-reference-collector.service" +grep -Fxq 'MemoryMax=768M' "$SCRIPT_DIR/polymarket-reference-collector.service"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh` around lines 1275 - 1277, Update the assertions in the service-memory test to also verify that polymarket-reference-collector.service contains the expected MemoryMax=768M setting, matching the existing shadow-unit envelope checks while retaining the current MemoryHigh assertions.
864-888: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover every newly enforced memory counter with rejection tests.
The mutations omit baseline
max/OOM counters and terminaloom_kill/oom_group_kill. Add these fields to a table-driven rejection matrix so each promotion-policy invariant is protected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh` around lines 864 - 888, Extend the rejection-test matrix in the memory-events policy checks to cover nonzero baseline max and oom counters, plus nonzero terminal oom_kill and oom_group_kill counters. Include each mutation in the same table-driven structure as the existing high, max, and oom cases, and assert that the policy rejects every generated gate fixture.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deployment/aliyun/polymarket-raw-ops-shadow-gate.sh`:
- Around line 751-753: Update the shutdown and terminal snapshot flow around
stable_memory_events_snapshot so the candidate is quiesced or frozen before
taking the final memory.events snapshot, or continue collecting counters
throughout process termination. Ensure high, max, and OOM events occurring
during shutdown are included before the gate evaluates the complete run.
---
Nitpick comments:
In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh`:
- Around line 1275-1277: Update the assertions in the service-memory test to
also verify that polymarket-reference-collector.service contains the expected
MemoryMax=768M setting, matching the existing shadow-unit envelope checks while
retaining the current MemoryHigh assertions.
- Around line 864-888: Extend the rejection-test matrix in the memory-events
policy checks to cover nonzero baseline max and oom counters, plus nonzero
terminal oom_kill and oom_group_kill counters. Include each mutation in the same
table-driven structure as the existing high, max, and oom cases, and assert that
the policy rejects every generated gate fixture.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ed1cb6f-ccf4-4e74-bb74-b674d4625150
📒 Files selected for processing (7)
deployment/aliyun/README.mddeployment/aliyun/polymarket-raw-ops-shadow-gate.shdeployment/aliyun/polymarket-reference-collector-shadow@.servicedeployment/aliyun/polymarket-reference-collector.servicedeployment/aliyun/polymarket-shadow-gate-policy.jqdeployment/aliyun/test-polymarket-raw-ops-control-plane.shdocs/reports/polymarket-shadow-memory-calibration-2026-07-16.md
c59f33d to
64f3e0d
Compare
64f3e0d to
cbb6424
Compare
|
Addressed the CodeRabbit terminal-snapshot review in cbb6424. The gate now freezes the exact shadow unit, verifies |
1 participant
Summary
memory.events highgrows or any MemoryMax/OOM counter is nonzeroValidation
bash -n deployment/aliyun/polymarket-raw-ops-shadow-gate.shbash deployment/aliyun/test-polymarket-raw-ops-control-plane.shgit diff --checkThis closes the gap found during the Tokyo one-hour shadow: business health was green, but the soft-limit counter was growing and was not represented in promotion evidence.
Summary by CodeRabbit
Enhancements
Documentation
Tests