Skip to content

Commit 03606ab

Browse files
committed
store: Pass dont_block in first poll attempt after a failure
Fixes #979. Discussion: #979 (comment)
1 parent 071ee0d commit 03606ab

File tree

2 files changed

+34
-15
lines changed

2 files changed

+34
-15
lines changed

lib/model/store.dart

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,7 +1399,13 @@ class UpdateMachine {
13991399
final GetEventsResult result;
14001400
try {
14011401
result = await getEvents(store.connection,
1402-
queueId: store.queueId, lastEventId: lastEventId);
1402+
queueId: store.queueId,
1403+
lastEventId: lastEventId,
1404+
// If the UI shows we're busy getting event-polling to work again,
1405+
// ask the server to tell us immediately that it's working again,
1406+
// rather than waiting for an event, which could take up to a minute
1407+
// in the case of a heartbeat event. See #979.
1408+
dontBlock: store.isRecoveringEventStream ? true : null);
14031409
if (_disposed) return;
14041410
} catch (e, stackTrace) {
14051411
if (_disposed) return;
@@ -1466,11 +1472,11 @@ class UpdateMachine {
14661472
// if we stayed at the max backoff interval; partway toward what would
14671473
// happen if we weren't backing off at all.
14681474
//
1469-
// But at least for [getEvents] requests, as here, it should be OK,
1470-
// because this is a long-poll. That means a typical successful request
1471-
// takes a long time to come back; in fact longer than our max backoff
1472-
// duration (which is 10 seconds). So if we're getting a mix of successes
1473-
// and failures, the successes themselves should space out the requests.
1475+
// Successful retries won't actually space out the requests, because retries
1476+
// are done with the `dont_block` param, asking the server to respond
1477+
// immediately instead of waiting through the long-poll period.
1478+
// (See comments on that code for why this behavior is helpful.)
1479+
// If server logs show pressure from too many requests, we can investigate.
14741480
_pollBackoffMachine = null;
14751481

14761482
store.isRecoveringEventStream = false;

test/model/store_test.dart

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -782,13 +782,14 @@ void main() {
782782
updateMachine.poll();
783783
}
784784

785-
void checkLastRequest({required int lastEventId}) {
785+
void checkLastRequest({required int lastEventId, bool expectDontBlock = false}) {
786786
check(connection.takeRequests()).single.isA<http.Request>()
787787
..method.equals('GET')
788788
..url.path.equals('/api/v1/events')
789789
..url.queryParameters.deepEquals({
790790
'queue_id': store.queueId,
791791
'last_event_id': lastEventId.toString(),
792+
if (expectDontBlock) 'dont_block': 'true',
792793
});
793794
}
794795

@@ -878,7 +879,7 @@ void main() {
878879
prepareError();
879880
updateMachine.debugAdvanceLoop();
880881
async.elapse(Duration.zero);
881-
checkLastRequest(lastEventId: 1);
882+
checkLastRequest(lastEventId: 1, expectDontBlock: false);
882883
check(store).isRecoveringEventStream.isTrue();
883884

884885
// Polling doesn't resume immediately; there's a timer.
@@ -893,7 +894,7 @@ void main() {
893894
HeartbeatEvent(id: 2),
894895
], queueId: null).toJson());
895896
async.flushTimers();
896-
checkLastRequest(lastEventId: 1);
897+
checkLastRequest(lastEventId: 1, expectDontBlock: true);
897898
check(updateMachine.lastEventId).equals(2);
898899
check(store).isRecoveringEventStream.isFalse();
899900
});
@@ -1032,10 +1033,12 @@ void main() {
10321033
await preparePoll(lastEventId: 1);
10331034
}
10341035

1035-
void pollAndFail(FakeAsync async, {bool shouldCheckRequest = true}) {
1036+
void pollAndFail(FakeAsync async, {bool shouldCheckRequest = true, bool expectDontBlock = false}) {
10361037
updateMachine.debugAdvanceLoop();
10371038
async.elapse(Duration.zero);
1038-
if (shouldCheckRequest) checkLastRequest(lastEventId: 1);
1039+
if (shouldCheckRequest) {
1040+
checkLastRequest(lastEventId: 1, expectDontBlock: expectDontBlock);
1041+
}
10391042
check(store).isRecoveringEventStream.isTrue();
10401043
}
10411044

@@ -1054,21 +1057,26 @@ void main() {
10541057
return awaitFakeAsync((async) async {
10551058
await prepare();
10561059

1060+
bool expectDontBlock = false; // (Not on the first attempt, but after that)
10571061
for (int i = 0; i < UpdateMachine.transientFailureCountNotifyThreshold; i++) {
10581062
prepareError();
1059-
pollAndFail(async);
1063+
pollAndFail(async, expectDontBlock: expectDontBlock);
1064+
expectDontBlock = true;
10601065
check(takeLastReportedError()).isNull();
10611066
async.flushTimers();
10621067
if (!identical(store, globalStore.perAccountSync(store.accountId))) {
10631068
// Store was reloaded.
10641069
updateFromGlobalStore();
10651070
updateMachine.debugPauseLoop();
10661071
updateMachine.poll();
1072+
// Loading indicator is cleared on successful /register;
1073+
// we don't need dont_block for the new queue's first poll.
1074+
expectDontBlock = false;
10671075
}
10681076
}
10691077

10701078
prepareError();
1071-
pollAndFail(async);
1079+
pollAndFail(async, expectDontBlock: expectDontBlock);
10721080
return check(takeLastReportedError()).isNotNull();
10731081
});
10741082
}
@@ -1077,21 +1085,26 @@ void main() {
10771085
return awaitFakeAsync((async) async {
10781086
await prepare();
10791087

1088+
bool expectDontBlock = false; // (Not on the first attempt, but after that)
10801089
for (int i = 0; i < UpdateMachine.transientFailureCountNotifyThreshold; i++) {
10811090
prepareError();
1082-
pollAndFail(async);
1091+
pollAndFail(async, expectDontBlock: expectDontBlock);
1092+
expectDontBlock = true;
10831093
check(takeLastReportedError()).isNull();
10841094
async.flushTimers();
10851095
if (!identical(store, globalStore.perAccountSync(store.accountId))) {
10861096
// Store was reloaded.
10871097
updateFromGlobalStore();
10881098
updateMachine.debugPauseLoop();
10891099
updateMachine.poll();
1100+
// Loading indicator is cleared on successful /register;
1101+
// we don't need dont_block for the new queue's first poll.
1102+
expectDontBlock = false;
10901103
}
10911104
}
10921105

10931106
prepareError();
1094-
pollAndFail(async);
1107+
pollAndFail(async, expectDontBlock: expectDontBlock);
10951108
// Still no error reported, even after the same number of iterations
10961109
// where other errors get reported (as [checkLateReported] checks).
10971110
check(takeLastReportedError()).isNull();

0 commit comments

Comments
 (0)