Skip to content

Commit f8b0e8b

Browse files
chrisbobbegnprice
authored andcommitted
store: Pass dont_block in first poll attempt after a failure
Fixes #979. Discussion: #979 (comment)
1 parent a451d58 commit f8b0e8b

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
@@ -1437,7 +1437,13 @@ class UpdateMachine {
14371437
final GetEventsResult result;
14381438
try {
14391439
result = await getEvents(store.connection,
1440-
queueId: store.queueId, lastEventId: lastEventId);
1440+
queueId: store.queueId,
1441+
lastEventId: lastEventId,
1442+
// If the UI shows we're busy getting event-polling to work again,
1443+
// ask the server to tell us immediately that it's working again,
1444+
// rather than waiting for an event, which could take up to a minute
1445+
// in the case of a heartbeat event. See #979.
1446+
dontBlock: store.isRecoveringEventStream ? true : null);
14411447
if (_disposed) return;
14421448
} catch (e, stackTrace) {
14431449
if (_disposed) return;
@@ -1504,11 +1510,11 @@ class UpdateMachine {
15041510
// if we stayed at the max backoff interval; partway toward what would
15051511
// happen if we weren't backing off at all.
15061512
//
1507-
// But at least for [getEvents] requests, as here, it should be OK,
1508-
// because this is a long-poll. That means a typical successful request
1509-
// takes a long time to come back; in fact longer than our max backoff
1510-
// duration (which is 10 seconds). So if we're getting a mix of successes
1511-
// and failures, the successes themselves should space out the requests.
1513+
// Successful retries won't actually space out the requests, because retries
1514+
// are done with the `dont_block` param, asking the server to respond
1515+
// immediately instead of waiting through the long-poll period.
1516+
// (See comments on that code for why this behavior is helpful.)
1517+
// If server logs show pressure from too many requests, we can investigate.
15121518
_pollBackoffMachine = null;
15131519

15141520
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;
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;
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)