Don't mark world state as stalled until a minimum time without progress is reached #1179
Conversation
…make progress for a specific number of requests *and* a minimum amount of time.
| @@ -131,6 +136,18 @@ public void shouldResetRequestsSinceProgressCountWhenProgressIsMade() { | |||
| assertThat(downloadState.getDownloadFuture()).isCompletedExceptionally(); | |||
| } | |||
|
|
|||
| @Test | |||
| public void shouldNotBeStalledWhenMaxRequestsReachedButNotMinimumTime() { | |||
There was a problem hiding this comment.
| public void shouldNotBeStalledWhenMaxRequestsReachedButNotMinimumTime() { | |
| public void shouldStallWhenMaxRequestsReachedAndTimeLimitExceeded() { |
There was a problem hiding this comment.
Looks like this test is checking for stalling? Worth adding the tests for the 2 other cases where it doesn't stall though (T/F and F/T for timeLimitHit/failedRequestsLimitHit).
There was a problem hiding this comment.
The test hits the maximum requests without progress limit and checks the download isn't considered stalled until the minimum time condition is also met. Tidied up the test name and implementation to make this clearer.
There was a problem hiding this comment.
Ensured the other combinations of time/requests reached/not-reached were covered.
|
|
||
| clock.stepMillis(MIN_MILLIS_BEFORE_STALLING + 1); | ||
| downloadState.requestComplete(false); | ||
| assertThat(downloadState.getDownloadFuture()).isCompletedExceptionally(); |
There was a problem hiding this comment.
Probably worth checking the exception here as well.
| downloadState.requestComplete(false); | ||
| assertThat(downloadState.getDownloadFuture()).isCompletedExceptionally(); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldNotAddRequestsAfterDownloadIsStalled() { |
There was a problem hiding this comment.
| public void shouldNotAddRequestsAfterDownloadIsStalled() { | |
| public void shouldNotAddRequestsAfterDownloadIsCompleted() { |
PR description
Delays marking a world state download as stalled until at least 5 minutes has passed since the last progress was made. This is in addition to requiring a minimum number of requests without progress.
Additionally increases the minimum number of requests without progress from 100 to 1000. Since restarting the world state download is expensive it's worth having high thresholds before giving up.