fix(deployment): raise Shadow memory limits - #848
Conversation
📝 WalkthroughWalkthroughThe Rust Binance LOB archiver receives higher systemd memory limits. The shadow gate now requires exact memory values and no drop-in overrides. The control-plane test validates the unit settings and gate checks. ChangesRust LOB memory enforcement
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c015f6bcf3
ℹ️ 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".
c015f6b to
b67ba81
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/binance-lob-archiver-rust`@.service:
- Around line 33-34: Keep MemoryHigh and MemoryMax in the shared
binance-lob-archiver-rust@.service template at the existing Spot values
(2500M/3200M). Apply 4400M/5000M only through the USD-M-specific unit or
drop-in, and update the related gate and contract test to assert the correct
per-market limits.
In `@deployment/aliyun/host-rust-lob-shadow-gate.sh`:
- Around line 518-525: Update the per-market validation around systemctl_value
and memory_max_bytes so Spot and USD-M use their distinct expected MemoryHigh
and MemoryMax values instead of requiring USD-M limits for every market. Adjust
DropInPaths validation to allow only the intended USD-M-specific drop-in while
preserving rejection of unexpected drop-ins for Spot.
In `@deployment/aliyun/test-rust-lob-control-plane.sh`:
- Around line 114-118: Update the gate-content assertion in the test to verify
the exact MemoryHigh value 4613734400, rather than only checking that the gate
reads MemoryHigh. Keep the existing assertions for DropInPaths and MemoryMax
unchanged.
🪄 Autofix
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 Plus
Run ID: 371b68f4-430b-466e-9fc3-443d47e23e26
📒 Files selected for processing (3)
deployment/aliyun/binance-lob-archiver-rust@.servicedeployment/aliyun/host-rust-lob-shadow-gate.shdeployment/aliyun/test-rust-lob-control-plane.sh
| MemoryHigh=4400M | ||
| MemoryMax=5000M |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Keep the memory increase USD-M-only.
binance-lob-archiver-rust@.service is a shared %i template. These values also apply to binance-lob-archiver-rust@spot.service, raising Spot from 2500M/3200M to 4400M/5000M. The PR objective excludes Spot limit changes.
Keep the shared template at the Spot values. Apply the higher values only to the USD-M-specific unit or drop-in, then update the gate and contract test for per-market values.
Proposed direction
-MemoryHigh=4400M
-MemoryMax=5000M
+MemoryHigh=2500M
+MemoryMax=3200M🤖 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/binance-lob-archiver-rust`@.service around lines 33 - 34,
Keep MemoryHigh and MemoryMax in the shared binance-lob-archiver-rust@.service
template at the existing Spot values (2500M/3200M). Apply 4400M/5000M only
through the USD-M-specific unit or drop-in, and update the related gate and
contract test to assert the correct per-market limits.
| [[ -z $(systemctl_value "$market" DropInPaths) ]] \ | ||
| || die "$market shadow service has an unexpected systemd drop-in" | ||
| [[ $(systemctl_value "$market" MemoryHigh) == 4613734400 ]] \ | ||
| || die "$market shadow service MemoryHigh differs from the gated template" | ||
| memory_max_bytes[$market]=$(require_uint "$(systemctl_value "$market" MemoryMax)" \ | ||
| "$market MemoryMax") | ||
| ((memory_max_bytes[$market] > 0)) || die "$market MemoryMax must be finite" | ||
| ((memory_max_bytes[$market] == 5242880000)) \ | ||
| || die "$market shadow service MemoryMax differs from the gated template" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Validate memory limits per market.
These comparisons require MemoryHigh=4613734400 and MemoryMax=5242880000 for every unit[$market]. The gate therefore requires the USD-M values for Spot and rejects the original Spot limits.
The DropInPaths check also rejects the USD-M-only drop-in described by the PR objective. Use separate Spot and USD-M expectations. Allow only the intended USD-M override.
🤖 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/host-rust-lob-shadow-gate.sh` around lines 518 - 525,
Update the per-market validation around systemctl_value and memory_max_bytes so
Spot and USD-M use their distinct expected MemoryHigh and MemoryMax values
instead of requiring USD-M limits for every market. Adjust DropInPaths
validation to allow only the intended USD-M-specific drop-in while preserving
rejection of unexpected drop-ins for Spot.
| grep -Fxq 'MemoryHigh=4400M' "$SHADOW_UNIT" | ||
| grep -Fxq 'MemoryMax=5000M' "$SHADOW_UNIT" | ||
| grep -Fq 'systemctl_value "$market" DropInPaths' "$GATE" | ||
| grep -Fq 'systemctl_value "$market" MemoryHigh' "$GATE" | ||
| grep -Fq 'memory_max_bytes[$market] == 5242880000' "$GATE" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the exact gated MemoryHigh value.
The test verifies that the gate reads MemoryHigh, but it does not verify 4613734400. A gate that still enforces the old value would pass this test.
Proposed test improvement
-grep -Fq 'systemctl_value "$market" MemoryHigh' "$GATE"
+grep -Fq 'MemoryHigh) == 4613734400' "$GATE"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| grep -Fxq 'MemoryHigh=4400M' "$SHADOW_UNIT" | |
| grep -Fxq 'MemoryMax=5000M' "$SHADOW_UNIT" | |
| grep -Fq 'systemctl_value "$market" DropInPaths' "$GATE" | |
| grep -Fq 'systemctl_value "$market" MemoryHigh' "$GATE" | |
| grep -Fq 'memory_max_bytes[$market] == 5242880000' "$GATE" | |
| grep -Fxq 'MemoryHigh=4400M' "$SHADOW_UNIT" | |
| grep -Fxq 'MemoryMax=5000M' "$SHADOW_UNIT" | |
| grep -Fq 'systemctl_value "$market" DropInPaths' "$GATE" | |
| grep -Fq 'MemoryHigh) == 4613734400' "$GATE" | |
| grep -Fq 'memory_max_bytes[$market] == 5242880000' "$GATE" |
🤖 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-rust-lob-control-plane.sh` around lines 114 - 118,
Update the gate-content assertion in the test to verify the exact MemoryHigh
value 4613734400, rather than only checking that the gate reads MemoryHigh. Keep
the existing assertions for DropInPaths and MemoryMax unchanged.
Change\n\nRaise the existing Shadow service template to
MemoryHigh=4400MandMemoryMax=5000M. Before admitting a Gate run, require those exact effective values for both Shadow instances and reject any systemd drop-in.\n\nAttempt 2 on #846 reached synced 570/570 with gap 0, then the old 2500M MemoryHigh boundary caused reclaim/socket throttling and saturated the ingest queue.\n\n## Issue relationship\n\nRefs #846\n\n## Validation\n\n-deployment/aliyun/test-rust-lob-control-plane.sh\n- Bash syntax and ShellCheck for the owning shell files\n-git diff --check\n\n## Runtime impact and rollback\n\nThis PR does not mutate runtime. A later candidate install replaces the existing Shadow template. Installing the prior release restores its prior template values; no persistent per-instance override remains.\n\n## Out of scope\n\nCollector logic, queue sizing, data-age gates, formal Gate, cutover, Live, and the draft shadow-soak tool.Summary by CodeRabbit
Bug Fixes
Tests