congestion: avoid overflows when calculating pacer budget#5390
congestion: avoid overflows when calculating pacer budget#5390marten-seemann merged 3 commits intomasterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5390 +/- ##
==========================================
+ Coverage 83.10% 83.15% +0.05%
==========================================
Files 159 159
Lines 19233 19252 +19
==========================================
+ Hits 15982 16008 +26
+ Misses 2623 2616 -7
Partials 628 628 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the pacer's budget calculation to prevent integer overflows when computing bandwidth-scaled time intervals. The changes improve both safety and performance (26% faster) by introducing a dedicated timeScaledBandwidth helper function that explicitly handles overflow conditions.
Key changes:
- Adds overflow protection in budget calculations using explicit checks
- Introduces
timeScaledBandwidthhelper to centralize bandwidth-time scaling logic - Removes the unused
infBandwidthconstant in favor ofmath.MaxUint64
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/congestion/pacer.go | Refactored budget calculation with new timeScaledBandwidth helper that includes overflow protection |
| internal/congestion/bandwidth.go | Removed unused infBandwidth constant |
| internal/congestion/pacer_test.go | Updated tests to use math.MaxUint64 instead of infBandwidth and added benchmark; includes debug print statement |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Tests are failing on Windows, like due to their subpar timers. |
d09ad76 to
d64e9bf
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
d64e9bf to
6baf0d0
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This should never happen for bandwidths even remotely possible in real-world scenarios, but it’s better to be safe.
6baf0d0 to
81beb17
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This should never happen for bandwidths even remotely possible in real-world scenarios, but it’s better to be safe.
This is exactly as fast (within measurement error) as what we had: