feat(server): Split bandwidth metering into ingress and egress#296
feat(server): Split bandwidth metering into ingress and egress#296
Conversation
lcian
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| let egress = self | ||
| .egress_estimate | ||
| .load(std::sync::atomic::Ordering::Relaxed); | ||
| ingress <= bps && egress <= bps |
There was a problem hiding this comment.
Rate limit checks directions independently, not total
High Severity
The check() method uses ingress <= bps && egress <= bps, which compares each direction independently against global_bps. The PR description explicitly states "Rate limiting still compares the total (ingress + egress) against the configured global_bps, preserving current behavior." The old code had a single combined accumulator, so the previous behavior was effectively limiting total bandwidth. The new logic allows up to 2× the configured global_bps in aggregate, breaking the intended rate limiting guarantee. The check needs to compare ingress + egress (or ingress.saturating_add(egress)) against bps.
Additional Locations (1)
There was a problem hiding this comment.
This is true, thinking about how this should behave/be configured
Convert MeteredPayloadStream from a struct to an enum with Ingress and Egress variants so upload and download bandwidth are tracked separately. BandwidthRateLimiter now maintains independent accumulators and EWMA estimates for each direction, emitting server.bandwidth.ingress.ewma and server.bandwidth.egress.ewma metrics. Rate limiting still uses the total of both directions. Also separate BandwidthRateLimiter construction from task spawning via a new start() method. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Apply global_bps as a cap on ingress and egress independently rather than on their sum, matching the physical network constraints where each direction has its own limit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lcian
force-pushed
the
ref/split-bandwidth-ingress-egress
branch
from
0b09b72 to
054007c
Compare
1 participant
Split
MeteredPayloadStreamintoIngressandEgressvariants so uploadand download bandwidth are tracked independently.
BandwidthRateLimiternowmaintains separate accumulators and EWMA estimates for each direction, emitting
server.bandwidth.ingress.ewmaandserver.bandwidth.egress.ewmametrics.Rate limiting still compares the total (ingress + egress) against the configured
global_bps, preserving current behavior.Also separates
BandwidthRateLimiterconstruction from background task spawningvia a new
start()method, called fromServices::spawn.Stacked on #293.