Skip to content

Commit

Permalink
Increase HttpConnectorMultiplexerIntegrationTest timeout to 20 second…
Browse files Browse the repository at this point in the history
…s from 10 seconds. 10 seconds seems to be close to the time it takes for the test to run, and flaked about 1 to 2%.

Also bump the timeout scaling factor for HttpConnector from 0.1 to 0.15, so that HttpConnector.READ_TIMEOUT_MS ends up being greater than HttpConnectorMultiplexer.FAILOVER_DELAY_MS in the test, as it is in production.

Also remove some unnecessary synchronization in the test.

RELNOTES: None.
PiperOrigin-RevId: 311142365
  • Loading branch information
ahumesky authored and Copybara-Service committed May 12, 2020
1 parent d74f059 commit b9786ca
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public HttpStream connect(List<URL> urls, Optional<Checksum> checksum) throws IO
* earlier mirrors are preferred. Each connector thread retries automatically on transient errors
* with exponential backoff. It vets the first 32kB of any payload before selecting a mirror in
* order to evade captive portals and avoid ultra-low-bandwidth servers. Even after this method
* returns the reliability doesn't stop. Each read operation wiil intercept timeouts and errors
* returns the reliability doesn't stop. Each read operation will intercept timeouts and errors
* and block until the connection can be renegotiated transparently right where it left off.
*
* @param urls mirrors by preference; each URL can be: file, http, or https
Expand Down Expand Up @@ -273,7 +273,7 @@ public void run() {
} catch (SocketTimeoutException e) {
// SocketTimeoutException derives from InterruptedIOException, but its occurrence
// is truly exceptional, so we handle it separately here. Failing to do so hides
// our exception from the user s/t they only see an inscrutable "thread
// our exception from the user so that they only see an inscrutable "thread
// interrupted" message instead.
synchronized (context) {
context.errors.add(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ public class HttpConnectorMultiplexerIntegrationTest {
@Rule
public final ExpectedException thrown = ExpectedException.none();

@Rule
public final Timeout globalTimeout = new Timeout(10000);
@Rule public final Timeout globalTimeout = new Timeout(20000);

private final ExecutorService executor = Executors.newFixedThreadPool(3);
private final ProxyHelper proxyHelper = mock(ProxyHelper.class);
Expand All @@ -73,7 +72,7 @@ public class HttpConnectorMultiplexerIntegrationTest {
private final Sleeper sleeper = mock(Sleeper.class);
private final Locale locale = Locale.US;
private final HttpConnector connector =
new HttpConnector(locale, eventHandler, proxyHelper, sleeper, 0.1f);
new HttpConnector(locale, eventHandler, proxyHelper, sleeper, 0.15f);
private final ProgressInputStream.Factory progressInputStreamFactory =
new ProgressInputStream.Factory(locale, clock, eventHandler);
private final HttpStream.Factory httpStreamFactory =
Expand Down Expand Up @@ -244,15 +243,14 @@ public Object call() throws Exception {

@Test
public void firstUrlSocketTimeout_secondOk() throws Exception {
final Phaser phaser = new Phaser(3);

try (ServerSocket server1 = new ServerSocket(0, 1, InetAddress.getByName(null));
ServerSocket server2 = new ServerSocket(0, 1, InetAddress.getByName(null))) {

@SuppressWarnings("unused")
Future<?> possiblyIgnoredError =
executor.submit(
() -> {
phaser.arriveAndAwaitAdvance();
try (Socket socket = server1.accept()) {
// Do nothing to cause SocketTimeoutException on client side.
}
Expand All @@ -263,7 +261,6 @@ public void firstUrlSocketTimeout_secondOk() throws Exception {
Future<?> possiblyIgnoredError2 =
executor.submit(
() -> {
phaser.arriveAndAwaitAdvance();
try (Socket socket = server2.accept()) {
readHttpRequest(socket.getInputStream());
sendLines(
Expand All @@ -277,8 +274,6 @@ public void firstUrlSocketTimeout_secondOk() throws Exception {
return null;
});

phaser.arriveAndAwaitAdvance();
phaser.arriveAndDeregister();
try (HttpStream stream =
multiplexer.connect(
ImmutableList.of(
Expand Down

0 comments on commit b9786ca

Please sign in to comment.