diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java index 5efe7b52d4975b..90a591873bf0af 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java @@ -18,6 +18,7 @@ import com.google.common.base.Optional; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; import com.google.common.io.ByteStreams; import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache; import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache.KeyType; @@ -35,8 +36,11 @@ import java.io.IOException; import java.io.InterruptedIOException; import java.io.OutputStream; +import java.net.SocketTimeoutException; import java.net.URI; import java.net.URL; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Map; @@ -199,21 +203,60 @@ public Path download( HttpConnectorMultiplexer multiplexer = new HttpConnectorMultiplexer(eventHandler, connector, httpStreamFactory, clock, sleeper); - // Connect to the best mirror and download the file, while reporting progress to the CLI. - semaphore.acquire(); + // Iterate over urls and download the file falling back to the next url if previous failed, + // while reporting progress to the CLI. boolean success = false; - try (HttpStream payload = multiplexer.connect(urls, checksum, authHeaders); - OutputStream out = destination.getOutputStream()) { - ByteStreams.copy(payload, out); - success = true; - } catch (InterruptedIOException e) { - throw new InterruptedException(); - } catch (IOException e) { - throw new IOException( - "Error downloading " + urls + " to " + destination + ": " + e.getMessage()); - } finally { - semaphore.release(); - eventHandler.post(new FetchEvent(urls.get(0).toString(), success)); + + List ioExceptions = ImmutableList.of(); + + for (URL url : urls) { + semaphore.acquire(); + + try (HttpStream payload = + multiplexer.connect(Collections.singletonList(url), checksum, authHeaders); + OutputStream out = destination.getOutputStream()) { + try { + ByteStreams.copy(payload, out); + } catch (SocketTimeoutException e) { + // SocketTimeoutExceptions are InterruptedIOExceptions; however they do not signify + // an external interruption, but simply a failed download due to some server timing + // out. So rethrow them as ordinary IOExceptions. + throw new IOException(e); + } + success = true; + break; + } catch (InterruptedIOException e) { + throw new InterruptedException(e.getMessage()); + } catch (IOException e) { + if (ioExceptions.isEmpty()) { + ioExceptions = new ArrayList<>(1); + } + ioExceptions.add(e); + eventHandler.handle( + Event.warn("Download from " + url + " failed: " + e.getClass() + " " + e.getMessage())); + continue; + } finally { + semaphore.release(); + eventHandler.post(new FetchEvent(url.toString(), success)); + } + } + + if (!success) { + final IOException exception = + new IOException( + "Error downloading " + + urls + + " to " + + destination + + (ioExceptions.isEmpty() + ? "" + : ": " + Iterables.getLast(ioExceptions).getMessage())); + + for (IOException cause : ioExceptions) { + exception.addSuppressed(cause); + } + + throw exception; } if (isCachingByProvidedChecksum) { diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD index e5873d4a0dc788..9d2f046d806c3c 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD @@ -21,6 +21,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib/bazel/repository/cache", "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader", + "//src/main/java/com/google/devtools/build/lib/vfs", "//src/test/java/com/google/devtools/build/lib:foundations_testutil", "//src/test/java/com/google/devtools/build/lib:test_runner", "//src/test/java/com/google/devtools/build/lib:testutil", diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloaderTestSuite.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloaderTestSuite.java index 6a479c2c39d134..60b07e56f52d51 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloaderTestSuite.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloaderTestSuite.java @@ -25,6 +25,7 @@ HttpConnectorMultiplexerIntegrationTest.class, HttpConnectorMultiplexerTest.class, HttpConnectorTest.class, + HttpDownloaderTest.class, HttpStreamTest.class, HttpUtilsTest.class, ProgressInputStreamTest.class, diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectorMultiplexerIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectorMultiplexerIntegrationTest.java index f5dccf879e797b..3b4c5af98a8d14 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectorMultiplexerIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectorMultiplexerIntegrationTest.java @@ -73,7 +73,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); + new HttpConnector(locale, eventHandler, proxyHelper, sleeper, 0.1f); private final ProgressInputStream.Factory progressInputStreamFactory = new ProgressInputStream.Factory(locale, clock, eventHandler); private final HttpStream.Factory httpStreamFactory = @@ -102,7 +102,7 @@ public void normalRequest() throws Exception { try (ServerSocket server1 = new ServerSocket(0, 1, InetAddress.getByName(null)); ServerSocket server2 = new ServerSocket(0, 1, InetAddress.getByName(null))) { for (final ServerSocket server : asList(server1, server2)) { - @SuppressWarnings("unused") + @SuppressWarnings("unused") Future possiblyIgnoredError = executor.submit( new Callable() { @@ -241,4 +241,52 @@ public Object call() throws Exception { HELLO_SHA256); } } + + @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. + } + return null; + }); + + @SuppressWarnings("unused") + Future possiblyIgnoredError2 = + executor.submit( + () -> { + phaser.arriveAndAwaitAdvance(); + try (Socket socket = server2.accept()) { + readHttpRequest(socket.getInputStream()); + sendLines( + socket, + "HTTP/1.1 200 OK", + "Date: Fri, 31 Dec 1999 23:59:59 GMT", + "Connection: close", + "", + "hello"); + } + return null; + }); + + phaser.arriveAndAwaitAdvance(); + phaser.arriveAndDeregister(); + try (HttpStream stream = + multiplexer.connect( + ImmutableList.of( + new URL(String.format("http://localhost:%d", server1.getLocalPort())), + new URL(String.format("http://localhost:%d", server2.getLocalPort()))), + HELLO_SHA256)) { + assertThat(toByteArray(stream)).isEqualTo("hello".getBytes(US_ASCII)); + } + } + } } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java new file mode 100644 index 00000000000000..e6ce8fb284dc73 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java @@ -0,0 +1,337 @@ +// Copyright 2019 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.bazel.repository.downloader; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.bazel.repository.downloader.DownloaderTestUtils.sendLines; +import static com.google.devtools.build.lib.bazel.repository.downloader.HttpParser.readHttpRequest; +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; + +import com.google.common.base.Optional; +import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache; +import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.vfs.DigestHashFunction; +import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException; +import com.google.devtools.build.lib.vfs.JavaIoFileSystem; +import com.google.devtools.build.lib.vfs.Path; +import java.io.DataInputStream; +import java.io.IOException; +import java.net.InetAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.net.SocketTimeoutException; +import java.net.URL; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import org.junit.After; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.rules.Timeout; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link HttpDownloader} */ +@RunWith(JUnit4.class) +public class HttpDownloaderTest { + + @Rule public final TemporaryFolder workingDir = new TemporaryFolder(); + + @Rule public final Timeout timeout = new Timeout(30, SECONDS); + + private final RepositoryCache repositoryCache = mock(RepositoryCache.class); + private final HttpDownloader httpDownloader = new HttpDownloader(repositoryCache); + + private final ExecutorService executor = Executors.newFixedThreadPool(2); + private final ExtendedEventHandler eventHandler = mock(ExtendedEventHandler.class); + private final JavaIoFileSystem fs; + + public HttpDownloaderTest() throws DefaultHashFunctionNotSetException { + try { + DigestHashFunction.setDefault(DigestHashFunction.SHA256); + } catch (DigestHashFunction.DefaultAlreadySetException e) { + // Do nothing. + } + fs = new JavaIoFileSystem(); + + // Scale timeouts down to make tests fast. + httpDownloader.setTimeoutScaling(0.1f); + } + + @After + public void after() { + executor.shutdown(); + } + + @Test + public void downloadFrom1UrlOk() throws IOException, InterruptedException { + try (ServerSocket server = new ServerSocket(0, 1, InetAddress.getByName(null))) { + @SuppressWarnings("unused") + Future possiblyIgnoredError = + executor.submit( + () -> { + try (Socket socket = server.accept()) { + readHttpRequest(socket.getInputStream()); + sendLines( + socket, + "HTTP/1.1 200 OK", + "Date: Fri, 31 Dec 1999 23:59:59 GMT", + "Connection: close", + "Content-Type: text/plain", + "Content-Length: 5", + "", + "hello"); + } + return null; + }); + + Path resultingFile = + httpDownloader.download( + Collections.singletonList( + new URL(String.format("http://localhost:%d/foo", server.getLocalPort()))), + Collections.emptyMap(), + Optional.absent(), + "testCanonicalId", + Optional.absent(), + fs.getPath(workingDir.newFile().getAbsolutePath()), + eventHandler, + Collections.emptyMap(), + "testRepo"); + + assertThat(new String(readFile(resultingFile), UTF_8)).isEqualTo("hello"); + } + } + + @Test + public void downloadFrom2UrlsFirstOk() throws IOException, InterruptedException { + 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( + () -> { + while (!executor.isShutdown()) { + try (Socket socket = server1.accept()) { + readHttpRequest(socket.getInputStream()); + sendLines( + socket, + "HTTP/1.1 200 OK", + "Date: Fri, 31 Dec 1999 23:59:59 GMT", + "Connection: close", + "Content-Type: text/plain", + "", + "content1"); + } + } + return null; + }); + + @SuppressWarnings("unused") + Future possiblyIgnoredError2 = + executor.submit( + () -> { + while (!executor.isShutdown()) { + try (Socket socket = server2.accept()) { + readHttpRequest(socket.getInputStream()); + sendLines( + socket, + "HTTP/1.1 200 OK", + "Date: Fri, 31 Dec 1999 23:59:59 GMT", + "Connection: close", + "Content-Type: text/plain", + "", + "content2"); + } + } + return null; + }); + + final List urls = new ArrayList<>(2); + urls.add(new URL(String.format("http://localhost:%d/foo", server1.getLocalPort()))); + urls.add(new URL(String.format("http://localhost:%d/foo", server2.getLocalPort()))); + + Path resultingFile = + httpDownloader.download( + urls, + Collections.emptyMap(), + Optional.absent(), + "testCanonicalId", + Optional.absent(), + fs.getPath(workingDir.newFile().getAbsolutePath()), + eventHandler, + Collections.emptyMap(), + "testRepo"); + + assertThat(new String(readFile(resultingFile), UTF_8)).isEqualTo("content1"); + } + } + + @Test + public void downloadFrom2UrlsFirstSocketTimeoutOnBodyReadSecondOk() + throws IOException, InterruptedException { + 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( + () -> { + Socket socket = server1.accept(); + readHttpRequest(socket.getInputStream()); + + sendLines( + socket, + "HTTP/1.1 200 OK", + "Date: Fri, 31 Dec 1999 23:59:59 GMT", + "Connection: close", + "Content-Type: text/plain", + "", + "content1"); + + // Never close the socket to cause SocketTimeoutException during body read on client + // side. + return null; + }); + + @SuppressWarnings("unused") + Future possiblyIgnoredError2 = + executor.submit( + () -> { + while (!executor.isShutdown()) { + try (Socket socket = server2.accept()) { + readHttpRequest(socket.getInputStream()); + sendLines( + socket, + "HTTP/1.1 200 OK", + "Date: Fri, 31 Dec 1999 23:59:59 GMT", + "Connection: close", + "Content-Type: text/plain", + "", + "content2"); + } + } + return null; + }); + + final List urls = new ArrayList<>(2); + urls.add(new URL(String.format("http://localhost:%d/foo", server1.getLocalPort()))); + urls.add(new URL(String.format("http://localhost:%d/foo", server2.getLocalPort()))); + + Path resultingFile = + httpDownloader.download( + urls, + Collections.emptyMap(), + Optional.absent(), + "testCanonicalId", + Optional.absent(), + fs.getPath(workingDir.newFile().getAbsolutePath()), + eventHandler, + Collections.emptyMap(), + "testRepo"); + + assertThat(new String(readFile(resultingFile), UTF_8)).isEqualTo("content2"); + } + } + + @Test + public void downloadFrom2UrlsBothSocketTimeoutDuringBodyRead() + throws IOException, InterruptedException { + 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( + () -> { + Socket socket = server1.accept(); + readHttpRequest(socket.getInputStream()); + + sendLines( + socket, + "HTTP/1.1 200 OK", + "Date: Fri, 31 Dec 1999 23:59:59 GMT", + "Connection: close", + "Content-Type: text/plain", + "", + "content1"); + + // Never close the socket to cause SocketTimeoutException during body read on client + // side. + return null; + }); + + @SuppressWarnings("unused") + Future possiblyIgnoredError2 = + executor.submit( + () -> { + Socket socket = server1.accept(); + readHttpRequest(socket.getInputStream()); + + sendLines( + socket, + "HTTP/1.1 200 OK", + "Date: Fri, 31 Dec 1999 23:59:59 GMT", + "Connection: close", + "Content-Type: text/plain", + "", + "content2"); + + // Never close the socket to cause SocketTimeoutException during body read on client + // side. + return null; + }); + + final List urls = new ArrayList<>(2); + urls.add(new URL(String.format("http://localhost:%d/foo", server1.getLocalPort()))); + urls.add(new URL(String.format("http://localhost:%d/foo", server2.getLocalPort()))); + + Path outputFile = fs.getPath(workingDir.newFile().getAbsolutePath()); + try { + httpDownloader.download( + urls, + Collections.emptyMap(), + Optional.absent(), + "testCanonicalId", + Optional.absent(), + outputFile, + eventHandler, + Collections.emptyMap(), + "testRepo"); + fail("Should have thrown"); + } catch (IOException expected) { + assertThat(expected.getSuppressed()).hasLength(2); + + for (Throwable suppressed : expected.getSuppressed()) { + assertThat(suppressed).isInstanceOf(IOException.class); + assertThat(suppressed).hasCauseThat().isInstanceOf(SocketTimeoutException.class); + } + } + } + } + + private static byte[] readFile(Path path) throws IOException { + final byte[] data = new byte[(int) path.getFileSize()]; + + try (DataInputStream stream = new DataInputStream(path.getInputStream())) { + stream.readFully(data); + } + + return data; + } +}