-
Notifications
You must be signed in to change notification settings - Fork 310
Reset backoff duration after successful connection. #871
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! One comment below, and I'll also mark this for Greg's review.
test/api/backoff_test.dart
Outdated
// probability < 1e-9. There are 2 * 11 assertions, and each one has a | ||
// failure probability < 1e-12; see below. | ||
// probability < 1e-9 for up to 1000 assertions. | ||
// There are 2 * [expectedMaxDurationsInMs.length] assertions, | ||
// and each one has a failure probability < 1e-12; see below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The part "for up to 1000 assertions" is new in this commit. What does it mean, and is there a logic change it corresponds to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commit allows an arbitrary number of expected durations to be passed, so the 1000
is here to convey how many expected values can be passed for the upper bound of 1e-9 to still hold.
After having a discussion with Greg, we will drop the testing part of this because this change is easy to verify, but testing is complicated. |
Fixes: zulip#554 Signed-off-by: Zixuan James Li <[email protected]>
Adapted from the text I wrote at zulip#554.
Thanks @PIG208, and thanks @chrisbobbe for the previous review! Merging, with an additional commit adding a long comment: |
This fixes #554, and supersedes #564.