Skip to content
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

backoff: Always wait a positive duration #726

Merged
merged 2 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions lib/api/backoff.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import 'dart:math';
class BackoffMachine {
BackoffMachine();

final double _firstDuration = 100;
final double _durationCeiling = 10 * 1000;
final double _base = 2;
static const double _firstDurationMs = 100;
static const double _durationCeilingMs = 10 * 1000;
static const double _base = 2;

DateTime? _startTime;

Expand All @@ -24,8 +24,8 @@ class BackoffMachine {
///
/// The popular exponential backoff strategy is to increase the duration
/// exponentially with the number of sleeps completed, with a base of 2,
/// until a ceiling is reached. E.g., if firstDuration is 100 and
/// durationCeiling is 10 * 1000 = 10000, the sequence is:
/// until a ceiling is reached. E.g., if the first duration is 100ms and
/// the ceiling is 10s = 10000ms, the sequence is, in ms:
///
/// 100, 200, 400, 800, 1600, 3200, 6400, 10000, 10000, 10000, ...
///
Expand All @@ -37,15 +37,22 @@ class BackoffMachine {
/// maximizes the range while preserving a capped exponential shape on
/// the expected value. Greg discusses this in more detail at:
/// https://github.com/zulip/zulip-mobile/pull/3841
///
/// The duration is always positive; [Duration] works in microseconds, so
/// we deviate from the idealized uniform distribution just by rounding
/// the smallest durations up to one microsecond instead of down to zero.
/// Because in the real world any delay takes nonzero time, this mainly
/// affects tests that use fake time, and keeps their behavior more realistic.
Future<void> wait() async {
_startTime ??= DateTime.now();

final duration =
final durationMs =
Random().nextDouble() // "Jitter"
* min(_durationCeiling,
_firstDuration * pow(_base, _waitsCompleted));
* min(_durationCeilingMs,
_firstDurationMs * pow(_base, _waitsCompleted));

await Future<void>.delayed(Duration(milliseconds: duration.round()));
await Future<void>.delayed(Duration(
microseconds: max(1, (1000 * durationMs).round())));

_waitsCompleted++;
}
Expand Down
21 changes: 21 additions & 0 deletions test/api/backoff_test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'dart:async';
import 'dart:math';

import 'package:checks/checks.dart';
import 'package:clock/clock.dart';
Expand Down Expand Up @@ -51,4 +52,24 @@ void main() {
check(maxFromAllTrials).isGreaterThan(expectedMax * 0.75);
}
});

test('BackoffMachine timeouts are always positive', () {
// Regression test for: https://github.com/zulip/zulip-flutter/issues/602
// This is a randomized test with a false-failure rate of zero.

// In the pre-#602 implementation, the first timeout was zero
// when a random number from [0, 100] was below 0.5.
// [numTrials] is chosen so that an implementation with that behavior
// will fail the test with probability 99%.
const hypotheticalFailureRate = 0.5 / 100;
const numTrials = 2 * ln10 / hypotheticalFailureRate;

awaitFakeAsync((async) async {
for (int i = 0; i < numTrials; i++) {
final duration = await measureWait(BackoffMachine().wait());
check(duration).isGreaterThan(Duration.zero);
}
check(async.pendingTimers).isEmpty();
});
});
}
Loading