Skip to content

Commit

Permalink
remote: Fix flaky ByteStreamUploaderTest.
Browse files Browse the repository at this point in the history
Running locally, I noticed that the multipleBlobsUploadShouldWork() test
is flaky (~1% of all runs).

The flakiness exists due to Future.get() being notified about completion
before the future's listeners are executed. We make use of future
listeners to remove an upload digest from an internal hashmap, after the
upload completed. Thus, checking for this map to be empty immediately
after uploadBlob (aka Future.get()) returned, could sometimes fail due
to the listeners not having executed yet.

The fix is to periodically poll the hashmap until all listeners have
executed.

RELNOTES: None.
PiperOrigin-RevId: 161507486
  • Loading branch information
buchgr authored and laszlocsomor committed Jul 11, 2017
1 parent 49e2e0d commit 7c22799
Showing 1 changed file with 11 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public void onCompleted() {
// This test should not have triggered any retries.
Mockito.verifyZeroInteractions(mockBackoff);

assertThat(uploader.uploadsInProgress()).isFalse();
blockUntilInternalStateConsistent(uploader);
}

@Test(timeout = 20000)
Expand Down Expand Up @@ -251,7 +251,7 @@ public void onCompleted() {

uploader.uploadBlobs(builders);

assertThat(uploader.uploadsInProgress()).isFalse();
blockUntilInternalStateConsistent(uploader);
}

@Test(timeout = 10000)
Expand Down Expand Up @@ -383,7 +383,7 @@ public void onCancel() {
assertThat(f1.isCancelled()).isTrue();
assertThat(f2.isCancelled()).isTrue();

assertThat(uploader.uploadsInProgress()).isFalse();
blockUntilInternalStateConsistent(uploader);
}

@Test(timeout = 10000)
Expand Down Expand Up @@ -521,4 +521,12 @@ public int getRetryAttempts() {
return retries;
}
}

private void blockUntilInternalStateConsistent(ByteStreamUploader uploader) throws Exception {
// Poll until all upload futures have been removed from the internal hash map. The polling is
// necessary, as listeners are executed after Future.get() calls are notified about completion.
while (uploader.uploadsInProgress()) {
Thread.sleep(1);
}
}
}

0 comments on commit 7c22799

Please sign in to comment.