Configurable RTOMax - fix #181 #303
Conversation
This change adds RTOMax to Config letting the user cap the retransmission timer without breaking compatibility. If RTOMax is 0 the current value of 60 seconds is used.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #303 +/- ##
==========================================
- Coverage 80.66% 80.66% -0.01%
==========================================
Files 49 49
Lines 4123 4133 +10
==========================================
+ Hits 3326 3334 +8
- Misses 652 653 +1
- Partials 145 146 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@daonb have you seen the issue go away if you just keep retransmitting? When testing the issue, I saw no progress happen sometimes even when RTO 3-6 happen |
|
@edaniels when congestion clears, the issue goes away and no data is lost. I remember once I had to wait 60 second for a key I hit to appear on the terminal. This will allow time critical low bandwidth data channel based peers to be more aggressive with their retransmission policy and keep the peers synced even in bad internet weather. |
|
gotcha! In that case we should get this in, especially since it's configurable. I have a feeling 181 is exposing more than one issue that goes beyond RTO, but it looks like we don't have any conclusive data yet. |
had to run the linter with --fix to get cleaner code
Description
This change adds RTOMax to Config letting the user cap the retransmission timer without breaking compatibility. If RTOMax is 0 the current value of 60 seconds is used.
Reference issue
Start fixing #181, needs another PR in webrtc to support this feature