diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageConfiguration.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageConfiguration.java index a0e699b07005..f4fb24f29d8b 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageConfiguration.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageConfiguration.java @@ -20,8 +20,13 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; import javax.annotation.Nullable; +import javax.net.ssl.SSLException; +import java.io.EOFException; +import java.net.SocketException; +import java.net.SocketTimeoutException; import java.util.Map; /** @@ -89,6 +94,17 @@ public abstract class CloudStorageConfiguration { */ public abstract boolean useUserProjectOnlyForRequesterPaysBuckets(); + /** + * Returns the set of HTTP error codes that will be retried, in addition to the normally + * retryable ones. + */ + public abstract ImmutableList retryableHttpCodes(); + + /** + * Returns the set of exceptions for which we'll try a channel reopen if maxChannelReopens + * is positive. + */ + public abstract ImmutableList> reopenableExceptions(); /** * Creates a new builder, initialized with the following settings: @@ -118,6 +134,10 @@ public static final class Builder { private @Nullable String userProject = null; // This of this as "clear userProject if not RequesterPays" private boolean useUserProjectOnlyForRequesterPaysBuckets = false; + private ImmutableList retryableHttpCodes = ImmutableList.of(500, 502, 503); + private ImmutableList> reopenableExceptions = + ImmutableList.>of( + SSLException.class, EOFException.class, SocketException.class, SocketTimeoutException.class); /** * Changes current working directory for new filesystem. This defaults to the root directory. @@ -186,6 +206,16 @@ public Builder autoDetectRequesterPays(boolean value) { return this; } + public Builder retryableHttpCodes(ImmutableList value) { + retryableHttpCodes = value; + return this; + } + + public Builder reopenableExceptions(ImmutableList> values) { + reopenableExceptions = values; + return this; + } + /** * Creates new instance without destroying builder. */ @@ -198,7 +228,9 @@ public CloudStorageConfiguration build() { blockSize, maxChannelReopens, userProject, - useUserProjectOnlyForRequesterPaysBuckets); + useUserProjectOnlyForRequesterPaysBuckets, + retryableHttpCodes, + reopenableExceptions); } Builder(CloudStorageConfiguration toModify) { @@ -210,6 +242,8 @@ public CloudStorageConfiguration build() { maxChannelReopens = toModify.maxChannelReopens(); userProject = toModify.userProject(); useUserProjectOnlyForRequesterPaysBuckets = toModify.useUserProjectOnlyForRequesterPaysBuckets(); + retryableHttpCodes = toModify.retryableHttpCodes(); + reopenableExceptions = toModify.reopenableExceptions(); } Builder() {} @@ -250,6 +284,12 @@ static private CloudStorageConfiguration fromMap(Builder builder, Map case "useUserProjectOnlyForRequesterPaysBuckets": builder.autoDetectRequesterPays((Boolean) entry.getValue()); break; + case "retryableHttpCodes": + builder.retryableHttpCodes((ImmutableList) entry.getValue()); + break; + case "reopenableExceptions": + builder.reopenableExceptions((ImmutableList>) entry.getValue()); + break; default: throw new IllegalArgumentException(entry.getKey()); } diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index 701a0d854ab2..ef025920818a 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -349,6 +349,7 @@ private SeekableByteChannel newReadChannel(Path path, Set cloudPath.getBlobId(), 0, maxChannelReopens, + cloudPath.getFileSystem().config(), userProject, blobSourceOptions.toArray(new BlobSourceOption[blobSourceOptions.size()])); } @@ -461,7 +462,8 @@ public boolean deleteIfExists(Path path) throws IOException { throw new CloudStoragePseudoDirectoryException(cloudPath); } - final CloudStorageRetryHandler retryHandler = new CloudStorageRetryHandler(CloudStorageUtil.getMaxChannelReopensFromPath(path)); + final CloudStorageRetryHandler retryHandler = new CloudStorageRetryHandler( + cloudPath.getFileSystem().config()); // Loop will terminate via an exception if all retries are exhausted while (true) { try { @@ -586,7 +588,8 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep throw new CloudStoragePseudoDirectoryException(toPath); } - final CloudStorageRetryHandler retryHandler = new CloudStorageRetryHandler(CloudStorageUtil.getMaxChannelReopensFromPath(source)); + final CloudStorageRetryHandler retryHandler = new CloudStorageRetryHandler( + fromPath.getFileSystem().config()); // Loop will terminate via an exception if all retries are exhausted while (true) { try { @@ -672,11 +675,12 @@ public void checkAccess(Path path, AccessMode... modes) throws IOException { } } - final CloudStorageRetryHandler retryHandler = new CloudStorageRetryHandler(CloudStorageUtil.getMaxChannelReopensFromPath(path)); + final CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path); + final CloudStorageRetryHandler retryHandler = new CloudStorageRetryHandler( + cloudPath.getFileSystem().config()); // Loop will terminate via an exception if all retries are exhausted while (true) { try { - CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path); boolean nullId; if (isNullOrEmpty(userProject)) { nullId = storage.get( @@ -725,11 +729,12 @@ public A readAttributes( } initStorage(); - final CloudStorageRetryHandler retryHandler = new CloudStorageRetryHandler(CloudStorageUtil.getMaxChannelReopensFromPath(path)); + final CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path); + final CloudStorageRetryHandler retryHandler = new CloudStorageRetryHandler( + cloudPath.getFileSystem().config()); // Loop will terminate via an exception if all retries are exhausted while (true) { try { - CloudStoragePath cloudPath = CloudStorageUtil.checkPath(path); BlobInfo blobInfo = null; try { BlobId blobId = cloudPath.getBlobId(); @@ -811,7 +816,8 @@ public DirectoryStream newDirectoryStream(final Path dir, final Filter 0) { - if ((throwable.getMessage() != null - && throwable.getMessage().contains("Connection closed prematurely")) - || throwable instanceof SSLException - || throwable instanceof EOFException - || throwable instanceof SocketException - || throwable instanceof SocketTimeoutException) { + for (Class reopenableException : config.reopenableExceptions()) { + if (reopenableException.isInstance(throwable)) { + return true; + } + } + if (throwable.getMessage() != null + && throwable.getMessage().contains("Connection closed prematurely")) { return true; } throwable = throwable.getCause(); diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageConfigurationTest.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageConfigurationTest.java index b4fc68d036a1..6548b446c52f 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageConfigurationTest.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageConfigurationTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.junit.Rule; @@ -26,6 +27,8 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import java.net.SocketTimeoutException; + /** * Unit tests for {@link CloudStorageConfiguration}. */ @@ -43,12 +46,16 @@ public void testBuilder() { .stripPrefixSlash(false) .usePseudoDirectories(false) .blockSize(666) + .retryableHttpCodes(ImmutableList.of(1,2,3)) + .reopenableExceptions(ImmutableList.>of(SocketTimeoutException.class)) .build(); assertThat(config.workingDirectory()).isEqualTo("/omg"); assertThat(config.permitEmptyPathComponents()).isTrue(); assertThat(config.stripPrefixSlash()).isFalse(); assertThat(config.usePseudoDirectories()).isFalse(); assertThat(config.blockSize()).isEqualTo(666); + assertThat(config.retryableHttpCodes()).isEqualTo(ImmutableList.of(1,2,3)); + assertThat(config.reopenableExceptions()).isEqualTo(ImmutableList.>of(SocketTimeoutException.class)); } @Test @@ -61,12 +68,16 @@ public void testFromMap() { .put("stripPrefixSlash", false) .put("usePseudoDirectories", false) .put("blockSize", 666) + .put("retryableHttpCodes", ImmutableList.of(1,2,3)) + .put("reopenableExceptions", ImmutableList.>of(SocketTimeoutException.class)) .build()); assertThat(config.workingDirectory()).isEqualTo("/omg"); assertThat(config.permitEmptyPathComponents()).isTrue(); assertThat(config.stripPrefixSlash()).isFalse(); assertThat(config.usePseudoDirectories()).isFalse(); assertThat(config.blockSize()).isEqualTo(666); + assertThat(config.retryableHttpCodes()).isEqualTo(ImmutableList.of(1,2,3)); + assertThat(config.reopenableExceptions()).isEqualTo(ImmutableList.>of(SocketTimeoutException.class)); } @Test @@ -74,4 +85,15 @@ public void testFromMap_badKey_throwsIae() { thrown.expect(IllegalArgumentException.class); CloudStorageConfiguration.fromMap(ImmutableMap.of("lol", "/omg")); } + + @Test + /** + * Spot check that our defaults are applied. + */ + public void testSomeDefaults() { + for (CloudStorageConfiguration config : ImmutableList.of(CloudStorageConfiguration.DEFAULT, CloudStorageConfiguration.builder().build())) { + assertThat(config.retryableHttpCodes()).contains(503); + assertThat(config.reopenableExceptions()).contains(SocketTimeoutException.class); + } + } } diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannelTest.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannelTest.java index 292ddff7e727..b8416e21572a 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannelTest.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageReadChannelTest.java @@ -67,7 +67,7 @@ public void before() throws IOException { when(gcsStorage.get(file, Storage.BlobGetOption.fields(Storage.BlobField.GENERATION, Storage.BlobField.SIZE))).thenReturn(metadata); when(gcsStorage.reader(file, Storage.BlobSourceOption.generationMatch(2L))).thenReturn(gcsChannel); when(gcsChannel.isOpen()).thenReturn(true); - chan = CloudStorageReadChannel.create(gcsStorage, file, 0, 1, ""); + chan = CloudStorageReadChannel.create(gcsStorage, file, 0, 1, CloudStorageConfiguration.DEFAULT, ""); verify(gcsStorage).get(eq(file), eq(Storage.BlobGetOption.fields(Storage.BlobField.GENERATION, Storage.BlobField.SIZE))); verify(gcsStorage).reader(eq(file), eq(Storage.BlobSourceOption.generationMatch(2L))); } @@ -208,7 +208,7 @@ public void testChannelPositionDoesNotGetTruncatedToInt() throws IOException { // Invoke CloudStorageReadChannel.create() to trigger a call to the private // CloudStorageReadChannel.innerOpen() method, which does a seek on our gcsChannel. - CloudStorageReadChannel.create(gcsStorage, file, startPosition, 1, ""); + CloudStorageReadChannel.create(gcsStorage, file, startPosition, 1, CloudStorageConfiguration.DEFAULT, ""); // Confirm that our position did not overflow during the seek in CloudStorageReadChannel.innerOpen() verify(gcsChannel).seek(captor.capture()); diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageRetryHandlerTest.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageRetryHandlerTest.java new file mode 100644 index 000000000000..c31648acd836 --- /dev/null +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageRetryHandlerTest.java @@ -0,0 +1,36 @@ +/* + * Copyright 2018 Google LLC + * + * 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.cloud.storage.contrib.nio; + +import org.junit.Test; +import com.google.cloud.storage.StorageException; +import com.google.common.collect.ImmutableList; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +import static com.google.common.truth.Truth.assertThat; + +@RunWith(JUnit4.class) +public class CloudStorageRetryHandlerTest { + + @Test + public void testIsRetryable() throws Exception { + CloudStorageConfiguration config = CloudStorageConfiguration.builder().retryableHttpCodes(ImmutableList.of(1,2,3)).build(); + CloudStorageRetryHandler retrier = new CloudStorageRetryHandler(config); + assertThat(retrier.isRetryable( new StorageException(1, "pretend error code 1"))).isTrue(); + } +} diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java index 7b11f24d87c3..ed5b687148f0 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java @@ -636,11 +636,17 @@ public void testListFilesInRootDirectory() throws IOException { // test absolute path, relative path. for (String check : new String[]{".", "/", ""}) { Path rootPath = fs.getPath(check); - List objectNames = new ArrayList<>(); + List pathsFound = new ArrayList<>(); for (Path path : Files.newDirectoryStream(rootPath)) { - objectNames.add(path.toString()); + // The returned paths will match the absolute-ness of the root path + // (this matches the behavior of the built-in UNIX file system). + assertWithMessage("Absolute/relative for " + check + ": ") + .that(path.isAbsolute()).isEqualTo(rootPath.isAbsolute()); + // To simplify the check that we found our files, we normalize here. + pathsFound.add(path.toAbsolutePath()); } - assertWithMessage("Listing " + check + ": ").that(objectNames).containsExactly(BIG_FILE, SML_FILE); + assertWithMessage("Listing " + check + ": ").that(pathsFound).containsExactly( + fs.getPath(BIG_FILE).toAbsolutePath(), fs.getPath(SML_FILE).toAbsolutePath()); } }