This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new 8783167752 fixed retry options for GCS write operations. fixes 8028
(#8038)
8783167752 is described below
commit 8783167752a8c31be3ce8e01b338c5290de2e493
Author: Bart Maertens <[email protected]>
AuthorDate: Fri Aug 21 09:50:28 2026 +0200
fixed retry options for GCS write operations. fixes 8028 (#8038)
---
.../apache/hop/vfs/gs/GoogleStorageFileSystem.java | 80 +++++++---
.../hop/vfs/gs/config/GoogleCloudConfig.java | 10 ++
.../hop/vfs/gs/config/GoogleCloudConfigPlugin.java | 25 ++++
.../gs/config/messages/messages_en_US.properties | 32 ++--
.../vfs/gs/GoogleStorageRetryBehaviourTest.java | 166 +++++++++++++++++++++
.../hop/vfs/gs/GoogleStorageRetrySettingsTest.java | 155 +++++++++++++++++++
6 files changed, 432 insertions(+), 36 deletions(-)
diff --git
a/plugins/tech/google/src/main/java/org/apache/hop/vfs/gs/GoogleStorageFileSystem.java
b/plugins/tech/google/src/main/java/org/apache/hop/vfs/gs/GoogleStorageFileSystem.java
index 72d2d7103a..0d6130a2c0 100644
---
a/plugins/tech/google/src/main/java/org/apache/hop/vfs/gs/GoogleStorageFileSystem.java
+++
b/plugins/tech/google/src/main/java/org/apache/hop/vfs/gs/GoogleStorageFileSystem.java
@@ -23,6 +23,7 @@ import com.google.api.gax.retrying.RetrySettings;
import com.google.cloud.http.HttpTransportOptions;
import com.google.cloud.storage.Storage;
import com.google.cloud.storage.StorageOptions;
+import com.google.cloud.storage.StorageRetryStrategy;
import com.google.storage.control.v2.StorageControlClient;
import com.google.storage.control.v2.StorageControlSettings;
import java.io.IOException;
@@ -83,30 +84,59 @@ public class GoogleStorageFileSystem extends
AbstractFileSystem {
GoogleCloudConfig config = GoogleCloudConfigSingleton.getConfig();
- RetrySettings retrySettings =
- StorageOptions.getDefaultRetrySettings().toBuilder()
- .setMaxAttempts(Integer.parseInt(config.getMaxAttempts()))
- .setInitialRetryDelay(
-
Duration.ofSeconds(Integer.parseInt(config.getInitialRetryDelay())))
-
.setRetryDelayMultiplier(Double.parseDouble(config.getRetryDelayMultiplier()))
-
.setMaxRetryDelay(Duration.ofSeconds(Integer.parseInt(config.getMaxRetryDelay())))
-
.setTotalTimeout(Duration.ofMinutes(Integer.parseInt(config.getTotalTimeout())))
- .setInitialRpcTimeout(
-
Duration.ofSeconds(Integer.parseInt(config.getInitialRpcTimeout())))
-
.setRpcTimeoutMultiplier(Double.parseDouble(config.getRpcTimeoutMultiplier()))
- // max RPC Timeout setting causes problems, disabled for now
- //
.setMaxRpcTimeout(Duration.ofSeconds(Integer.parseInt(config.getMaxRpcTimeout())))
- .build();
-
- StorageOptions.Builder optionsBuilder = StorageOptions.newBuilder();
+ StorageOptions.Builder optionsBuilder = buildStorageOptions(config);
optionsBuilder.setCredentials(
GoogleStorageFileSystemConfigBuilder.getInstance().getGoogleCredentials(fileSystemOptions));
- optionsBuilder.setRetrySettings(retrySettings);
- optionsBuilder.setTransportOptions(buildTransportOptions(config));
return storage = optionsBuilder.build().getService();
}
+ /**
+ * Assemble everything about the client that depends only on the
configuration. Kept separate from
+ * {@link #setupStorage()} - which additionally needs credentials and a live
service - so the
+ * wiring can be exercised from a test against a local endpoint.
+ */
+ static StorageOptions.Builder buildStorageOptions(GoogleCloudConfig config) {
+ return StorageOptions.newBuilder()
+ .setRetrySettings(buildRetrySettings(config))
+ .setTransportOptions(buildTransportOptions(config))
+ .setStorageRetryStrategy(selectRetryStrategy(config));
+ }
+
+ /**
+ * The GCS client only retries calls it considers idempotent. Object create,
delete and
+ * upload-session-start carry no preconditions here, so they are classified
non-idempotent and are
+ * never retried - whatever the configured number of attempts says. The
uniform strategy drops
+ * that distinction and retries writes too; see {@link
+ * GoogleCloudConfig#getRetryNonIdempotentOperations()} for why it is opt-in.
+ */
+ static StorageRetryStrategy selectRetryStrategy(GoogleCloudConfig config) {
+ return Boolean.TRUE.equals(config.getRetryNonIdempotentOperations())
+ ? StorageRetryStrategy.getUniformStorageRetryStrategy()
+ : StorageRetryStrategy.getDefaultStorageRetryStrategy();
+ }
+
+ static RetrySettings buildRetrySettings(GoogleCloudConfig config) {
+ long initialRpcTimeout = Const.toLong(config.getInitialRpcTimeout(), 50);
+ // gax rejects a max RPC timeout below the initial one with an
IllegalStateException while the
+ // client is being built, taking down all GCS access. Raise the ceiling to
whatever the user
+ // explicitly asked for as a starting point rather than failing to connect
at all.
+ long maxRpcTimeout = Math.max(Const.toLong(config.getMaxRpcTimeout(), 50),
initialRpcTimeout);
+
+ return StorageOptions.getDefaultRetrySettings().toBuilder()
+ .setMaxAttempts(Const.toInt(config.getMaxAttempts(), 6))
+
.setInitialRetryDelay(Duration.ofSeconds(Const.toLong(config.getInitialRetryDelay(),
1)))
+
.setRetryDelayMultiplier(Const.toDouble(config.getRetryDelayMultiplier(), 2.0))
+
.setMaxRetryDelay(Duration.ofSeconds(Const.toLong(config.getMaxRetryDelay(),
32)))
+ // Minutes, unlike every other duration here - kept that way so
upgrading does not silently
+ // shorten existing configurations by a factor of 60. The label spells
out the unit.
+
.setTotalTimeout(Duration.ofMinutes(Const.toLong(config.getTotalTimeout(), 50)))
+ .setInitialRpcTimeout(Duration.ofSeconds(initialRpcTimeout))
+
.setRpcTimeoutMultiplier(Const.toDouble(config.getRpcTimeoutMultiplier(), 1.0))
+ .setMaxRpcTimeout(Duration.ofSeconds(maxRpcTimeout))
+ .build();
+ }
+
static HttpTransportOptions buildTransportOptions(GoogleCloudConfig config) {
int connectTimeoutMs = Const.toInt(config.getConnectionTimeout(), 20) *
1000;
int readTimeoutMs = Const.toInt(config.getReadTimeout(), 20) * 1000;
@@ -160,13 +190,21 @@ public class GoogleStorageFileSystem extends
AbstractFileSystem {
if (storageControlClient != null) {
return storageControlClient;
}
- StorageControlSettings settings =
+ RetrySettings retrySettings =
buildRetrySettings(GoogleCloudConfigSingleton.getConfig());
+ StorageControlSettings.Builder builder =
StorageControlSettings.newBuilder()
.setCredentialsProvider(
FixedCredentialsProvider.create(
GoogleStorageFileSystemConfigBuilder.getInstance()
- .getGoogleCredentials(fileSystemOptions)))
- .build();
+ .getGoogleCredentials(fileSystemOptions)));
+ // This client was left on the library defaults, so the configured retry
behaviour never
+ // reached HNS folder operations.
+ builder.applyToAllUnaryMethods(
+ callSettings -> {
+ callSettings.setRetrySettings(retrySettings);
+ return null;
+ });
+ StorageControlSettings settings = builder.build();
storageControlClient = StorageControlClient.create(settings);
return storageControlClient;
}
diff --git
a/plugins/tech/google/src/main/java/org/apache/hop/vfs/gs/config/GoogleCloudConfig.java
b/plugins/tech/google/src/main/java/org/apache/hop/vfs/gs/config/GoogleCloudConfig.java
index 56b4b579da..8845f6bfa2 100644
---
a/plugins/tech/google/src/main/java/org/apache/hop/vfs/gs/config/GoogleCloudConfig.java
+++
b/plugins/tech/google/src/main/java/org/apache/hop/vfs/gs/config/GoogleCloudConfig.java
@@ -39,6 +39,14 @@ public class GoogleCloudConfig {
private String connectionTimeout;
private String readTimeout;
+ /**
+ * Retry operations that the GCS client considers non-idempotent (object
create, delete, starting
+ * an upload). The client only retries idempotent calls by default, so
without this the retry
+ * settings above never apply to writes. Off by default: a retried delete
can come back 404 once
+ * the first attempt succeeded server-side, and a retried create is a
last-write-wins overwrite.
+ */
+ private Boolean retryNonIdempotentOperations;
+
/** Cache TTL in seconds for list-result caching (same as S3/MinIO/Azure). */
private String cacheTtlSeconds;
@@ -54,6 +62,7 @@ public class GoogleCloudConfig {
maxRpcTimeout = "50";
connectionTimeout = "20";
readTimeout = "20";
+ retryNonIdempotentOperations = false;
cacheTtlSeconds = "5";
}
@@ -71,6 +80,7 @@ public class GoogleCloudConfig {
maxRpcTimeout = config.maxRpcTimeout;
connectionTimeout = config.connectionTimeout;
readTimeout = config.readTimeout;
+ retryNonIdempotentOperations = config.retryNonIdempotentOperations;
cacheTtlSeconds = config.cacheTtlSeconds;
}
}
diff --git
a/plugins/tech/google/src/main/java/org/apache/hop/vfs/gs/config/GoogleCloudConfigPlugin.java
b/plugins/tech/google/src/main/java/org/apache/hop/vfs/gs/config/GoogleCloudConfigPlugin.java
index 2d3c449866..cd84792bd2 100644
---
a/plugins/tech/google/src/main/java/org/apache/hop/vfs/gs/config/GoogleCloudConfigPlugin.java
+++
b/plugins/tech/google/src/main/java/org/apache/hop/vfs/gs/config/GoogleCloudConfigPlugin.java
@@ -74,6 +74,8 @@ public class GoogleCloudConfigPlugin implements
IConfigOptions, IGuiPluginCompos
"10900-google-cloud-service-connect-timeout";
private static final String WIDGET_ID_GOOGLE_CLOUD_SERVICE_READ_TIMEOUT =
"1100-google-cloud-service-read-timeout";
+ private static final String
WIDGET_ID_GOOGLE_CLOUD_SERVICE_RETRY_NON_IDEMPOTENT =
+ "11000-google-cloud-service-retry-non-idempotent";
private static final String WIDGET_ID_GOOGLE_CLOUD_CACHE_TTL_SECONDS =
"11100-google-cloud-cache-ttl-seconds";
@@ -188,6 +190,15 @@ public class GoogleCloudConfigPlugin implements
IConfigOptions, IGuiPluginCompos
toolTip = "i18n::GoogleCloudPlugin.ReadTimeout.Description")
private String readTimeout;
+ @GuiWidgetElement(
+ id = WIDGET_ID_GOOGLE_CLOUD_SERVICE_RETRY_NON_IDEMPOTENT,
+ parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
+ type = GuiElementType.CHECKBOX,
+ variables = false,
+ label = "i18n::GoogleCloudPlugin.RetryNonIdempotentOperations.Label",
+ toolTip =
"i18n::GoogleCloudPlugin.RetryNonIdempotentOperations.Description")
+ private Boolean retryNonIdempotentOperations;
+
@GuiWidgetElement(
id = WIDGET_ID_GOOGLE_CLOUD_CACHE_TTL_SECONDS,
parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
@@ -218,6 +229,7 @@ public class GoogleCloudConfigPlugin implements
IConfigOptions, IGuiPluginCompos
instance.maxRpcTimeout = config.getMaxRpcTimeout();
instance.connectTimeout = config.getConnectionTimeout();
instance.readTimeout = config.getReadTimeout();
+ instance.retryNonIdempotentOperations =
config.getRetryNonIdempotentOperations();
instance.cacheTtlSeconds = config.getCacheTtlSeconds();
return instance;
@@ -309,6 +321,14 @@ public class GoogleCloudConfigPlugin implements
IConfigOptions, IGuiPluginCompos
changed = true;
}
+ if (retryNonIdempotentOperations != null) {
+ config.setRetryNonIdempotentOperations(retryNonIdempotentOperations);
+ log.logBasic(
+ "Google Cloud service retry of non-idempotent operations set to "
+ + retryNonIdempotentOperations);
+ changed = true;
+ }
+
if (cacheTtlSeconds != null) {
config.setCacheTtlSeconds(cacheTtlSeconds);
log.logBasic("Google Cloud list cache TTL (seconds) set to " +
cacheTtlSeconds);
@@ -396,6 +416,11 @@ public class GoogleCloudConfigPlugin implements
IConfigOptions, IGuiPluginCompos
readTimeout = ((TextVar) control).getText();
GoogleCloudConfigSingleton.getConfig().setReadTimeout(readTimeout);
break;
+ case WIDGET_ID_GOOGLE_CLOUD_SERVICE_RETRY_NON_IDEMPOTENT:
+ retryNonIdempotentOperations = ((Button) control).getSelection();
+ GoogleCloudConfigSingleton.getConfig()
+ .setRetryNonIdempotentOperations(retryNonIdempotentOperations);
+ break;
case WIDGET_ID_GOOGLE_CLOUD_CACHE_TTL_SECONDS:
cacheTtlSeconds = ((TextVar) control).getText();
GoogleCloudConfigSingleton.getConfig().setCacheTtlSeconds(cacheTtlSeconds);
diff --git
a/plugins/tech/google/src/main/resources/org/apache/hop/vfs/gs/config/messages/messages_en_US.properties
b/plugins/tech/google/src/main/resources/org/apache/hop/vfs/gs/config/messages/messages_en_US.properties
index 23c0b2d185..f4429ffff3 100644
---
a/plugins/tech/google/src/main/resources/org/apache/hop/vfs/gs/config/messages/messages_en_US.properties
+++
b/plugins/tech/google/src/main/resources/org/apache/hop/vfs/gs/config/messages/messages_en_US.properties
@@ -21,25 +21,27 @@ GoogleCloudPlugin.AccountKeyFile.Description=The path to a
Google Cloud service
GoogleCloudPlugin.AccountKeyFile.Label=Account key file
GoogleCloudPlugin.GuiPlugin.Description=Google Cloud
GoogleCloudPlugin.MaxAttempts.Label=Max number of attempts
-GoogleCloudPlugin.MaxAttempts.Description=Max number of attempts
-GoogleCloudPlugin.InitialRetryDelay.Label=Initial retry delay
-GoogleCloudPlugin.InitialRetryDelay.Description=Initial retry delay
+GoogleCloudPlugin.MaxAttempts.Description=Maximum number of attempts for a
single request, including the first one
+GoogleCloudPlugin.InitialRetryDelay.Label=Initial retry delay (seconds)
+GoogleCloudPlugin.InitialRetryDelay.Description=Delay before the first retry,
in seconds
GoogleCloudPlugin.RetryDelayMultiplier.Label=Retry delay multiplier
GoogleCloudPlugin.RetryDelayMultiplier.Description=Retry delay multiplier
-GoogleCloudPlugin.MaxRetryDelay.Label=Maximum retry delay
-GoogleCloudPlugin.MaxRetryDelay.Description=Maximum retry delay
-GoogleCloudPlugin.TotalTimeout.Label=Total Timeout
-GoogleCloudPlugin.TotalTimeout.Description=Total Timeout
-GoogleCloudPlugin.InitialRpcTimeout.Label=Initial RPC Timeout
-GoogleCloudPlugin.InitialRpcTimeout.Description=Initial RPC Timeout
+GoogleCloudPlugin.MaxRetryDelay.Label=Maximum retry delay (seconds)
+GoogleCloudPlugin.MaxRetryDelay.Description=Ceiling for the retry delay once
the multiplier has been applied, in seconds
+GoogleCloudPlugin.TotalTimeout.Label=Total timeout (minutes)
+GoogleCloudPlugin.TotalTimeout.Description=Deadline for a request including
all of its retries, in minutes
+GoogleCloudPlugin.InitialRpcTimeout.Label=Initial RPC timeout (seconds)
+GoogleCloudPlugin.InitialRpcTimeout.Description=Timeout for the first attempt,
in seconds. Raised to the max RPC timeout if that is lower.
GoogleCloudPlugin.RpcTimeoutMultiplier.Label=RPC Timeout Multiplier
GoogleCloudPlugin.RpcTimeoutMultiplier.Description=RPC Timeout Multiplier
-GoogleCloudPlugin.MaxRpcTimeout.Label=Max RPC Timeout
-GoogleCloudPlugin.MaxRpcTimeout.Description=Max RPC Timeout
-GoogleCloudPlugin.ConnectTimeout.Label=Connect Timeout
-GoogleCloudPlugin.ConnectTimeout.Description=Connect Timeout
-GoogleCloudPlugin.ReadTimeout.Label=Read Timeout
-GoogleCloudPlugin.ReadTimeout.Description=Read Timeout
+GoogleCloudPlugin.MaxRpcTimeout.Label=Max RPC timeout (seconds)
+GoogleCloudPlugin.MaxRpcTimeout.Description=Ceiling for a single attempt
timeout, in seconds. Never lower than the initial RPC timeout.
+GoogleCloudPlugin.ConnectTimeout.Label=Connect timeout (seconds)
+GoogleCloudPlugin.ConnectTimeout.Description=Socket connect timeout, in seconds
+GoogleCloudPlugin.ReadTimeout.Label=Read timeout (seconds)
+GoogleCloudPlugin.ReadTimeout.Description=Socket read timeout, in seconds
+GoogleCloudPlugin.RetryNonIdempotentOperations.Label=Retry non-idempotent
operations
+GoogleCloudPlugin.RetryNonIdempotentOperations.Description=Also retry writes
(create, delete, starting an upload). Google Cloud Storage only retries reads
by default, so without this the retry settings above do not apply to writing
files. Note that a retried delete can report "not found" when the first attempt
already succeeded, and a retried create overwrites.
GoogleCloudPlugin.CacheTtlSeconds.Label=Cache TTL (seconds)
GoogleCloudPlugin.CacheTtlSeconds.Description=How long to cache folder listing
results (in seconds, default 5). If not set, falls back to 10 seconds.
GoogleCloudPlugin.ScanFolderForLastModificationDate.Label=Scan folders to find
last modified data
diff --git
a/plugins/tech/google/src/test/java/org/apache/hop/vfs/gs/GoogleStorageRetryBehaviourTest.java
b/plugins/tech/google/src/test/java/org/apache/hop/vfs/gs/GoogleStorageRetryBehaviourTest.java
new file mode 100644
index 0000000000..3dc9ca10fc
--- /dev/null
+++
b/plugins/tech/google/src/test/java/org/apache/hop/vfs/gs/GoogleStorageRetryBehaviourTest.java
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.hop.vfs.gs;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import com.google.api.gax.retrying.RetrySettings;
+import com.google.cloud.NoCredentials;
+import com.google.cloud.storage.BlobId;
+import com.google.cloud.storage.BlobInfo;
+import com.google.cloud.storage.Storage;
+import com.google.cloud.storage.StorageException;
+import com.sun.net.httpserver.HttpServer;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.net.InetSocketAddress;
+import java.nio.charset.StandardCharsets;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.hop.vfs.gs.config.GoogleCloudConfig;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.threeten.bp.Duration;
+
+/**
+ * Counts the HTTP requests the storage client actually makes against a local
endpoint that rejects
+ * the first few calls, which is the only way to tell a configured retry from
an ignored one.
+ *
+ * <p>Google Cloud Storage classifies a call as idempotent only when it
carries a precondition.
+ * Reading and listing always qualify; creating, deleting and starting an
upload do not, and the
+ * client silently refuses to retry those no matter how many attempts are
configured. That is what
+ * {@link GoogleCloudConfig#getRetryNonIdempotentOperations()} exists to
change.
+ */
+class GoogleStorageRetryBehaviourTest {
+
+ /** Requests rejected before the endpoint starts answering normally. */
+ private static final int REJECTED_REQUESTS = 3;
+
+ private static final String REJECTION =
+ "{\"error\":{\"code\":429,\"message\":\"The rate of change requests to
the object exceeds "
+ + "the rate
limit.\",\"errors\":[{\"domain\":\"usageLimits\",\"reason\":"
+ + "\"rateLimitExceeded\",\"message\":\"rate limit exceeded\"}]}}";
+
+ private static final String OBJECT =
+ "{\"kind\":\"storage#object\",\"bucket\":\"bucket\",\"name\":\"object\","
+ + "\"generation\":\"1\",\"metageneration\":\"1\",\"size\":\"0\"}";
+
+ private HttpServer server;
+ private final AtomicInteger requests = new AtomicInteger();
+
+ @BeforeEach
+ void startEndpoint() throws IOException {
+ server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0);
+ server.createContext(
+ "/",
+ exchange -> {
+ boolean reject = requests.incrementAndGet() <= REJECTED_REQUESTS;
+ byte[] body = (reject ? REJECTION :
OBJECT).getBytes(StandardCharsets.UTF_8);
+ exchange.getResponseHeaders().add("Content-Type", "application/json;
charset=UTF-8");
+ exchange.sendResponseHeaders(reject ? 429 : 200, body.length);
+ try (OutputStream out = exchange.getResponseBody()) {
+ out.write(body);
+ }
+ });
+ server.start();
+ }
+
+ @AfterEach
+ void stopEndpoint() {
+ server.stop(0);
+ }
+
+ @Test
+ void readsAreRetriedWithTheDefaultStrategy() {
+ Storage storage = storageWith(new GoogleCloudConfig());
+
+ assertNotNull(storage.get(BlobId.of("bucket", "object")));
+ assertEquals(
+ REJECTED_REQUESTS + 1, requests.get(), "a read should be retried until
it succeeds");
+ }
+
+ /** The reported bug: attempts are configured, and a write still gives up
immediately. */
+ @Test
+ void writesAreNotRetriedWithTheDefaultStrategy() {
+ Storage storage = storageWith(new GoogleCloudConfig());
+
+ assertThrows(
+ StorageException.class,
+ () ->
+ storage.create(BlobInfo.newBuilder("bucket", "object").build(),
new byte[] {1, 2, 3}));
+ assertEquals(1, requests.get(), "the default strategy never retries a
create");
+
+ requests.set(0);
+ assertThrows(StorageException.class, () ->
storage.delete(BlobId.of("bucket", "object")));
+ assertEquals(1, requests.get(), "the default strategy never retries a
delete");
+ }
+
+ @Test
+ void writesAreRetriedOnceNonIdempotentRetriesAreEnabled() {
+ GoogleCloudConfig config = new GoogleCloudConfig();
+ config.setRetryNonIdempotentOperations(true);
+ Storage storage = storageWith(config);
+
+ assertNotNull(
+ storage.create(BlobInfo.newBuilder("bucket", "object").build(), new
byte[] {1, 2, 3}));
+ assertEquals(REJECTED_REQUESTS + 1, requests.get(), "a create should now
be retried");
+
+ requests.set(0);
+ storage.delete(BlobId.of("bucket", "object"));
+ assertEquals(REJECTED_REQUESTS + 1, requests.get(), "a delete should now
be retried");
+ }
+
+ @Test
+ void retriesStopAtTheConfiguredNumberOfAttempts() {
+ GoogleCloudConfig config = new GoogleCloudConfig();
+ config.setRetryNonIdempotentOperations(true);
+ config.setMaxAttempts("2");
+ Storage storage = storageWith(config);
+
+ assertThrows(
+ StorageException.class,
+ () ->
+ storage.create(BlobInfo.newBuilder("bucket", "object").build(),
new byte[] {1, 2, 3}));
+ assertEquals(2, requests.get(), "two attempts were configured, so two
requests");
+ }
+
+ /**
+ * Builds the client the way {@link GoogleStorageFileSystem#setupStorage()}
does, pointed at the
+ * local endpoint. The retry delays are collapsed to milliseconds so the
test stays fast; the
+ * configured attempt count - the thing under test - is left alone. The
delay values themselves
+ * are covered by {@link GoogleStorageRetrySettingsTest}.
+ */
+ private Storage storageWith(GoogleCloudConfig config) {
+ RetrySettings prompt =
+ GoogleStorageFileSystem.buildRetrySettings(config).toBuilder()
+ .setInitialRetryDelay(Duration.ofMillis(1))
+ .setMaxRetryDelay(Duration.ofMillis(2))
+ .build();
+
+ return GoogleStorageFileSystem.buildStorageOptions(config)
+ .setRetrySettings(prompt)
+ .setHost("http://127.0.0.1:"; + server.getAddress().getPort())
+ .setProjectId("hop-test")
+ .setCredentials(NoCredentials.getInstance())
+ .build()
+ .getService();
+ }
+}
diff --git
a/plugins/tech/google/src/test/java/org/apache/hop/vfs/gs/GoogleStorageRetrySettingsTest.java
b/plugins/tech/google/src/test/java/org/apache/hop/vfs/gs/GoogleStorageRetrySettingsTest.java
new file mode 100644
index 0000000000..67ead57f3e
--- /dev/null
+++
b/plugins/tech/google/src/test/java/org/apache/hop/vfs/gs/GoogleStorageRetrySettingsTest.java
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.hop.vfs.gs;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+import com.google.api.gax.retrying.RetrySettings;
+import com.google.cloud.storage.StorageRetryStrategy;
+import java.time.Duration;
+import org.apache.hop.vfs.gs.config.GoogleCloudConfig;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The values in the Google Cloud options dialog must actually reach the
client's {@link
+ * RetrySettings}. Companion to {@link GoogleStorageTransportOptionsTest},
which covers the socket
+ * timeouts on the same dialog.
+ */
+class GoogleStorageRetrySettingsTest {
+
+ @Test
+ void defaultConfigMatchesTheShippedDefaults() {
+ RetrySettings settings = GoogleStorageFileSystem.buildRetrySettings(new
GoogleCloudConfig());
+
+ assertEquals(6, settings.getMaxAttempts());
+ assertEquals(Duration.ofSeconds(1),
settings.getInitialRetryDelayDuration());
+ assertEquals(2.0, settings.getRetryDelayMultiplier());
+ assertEquals(Duration.ofSeconds(32), settings.getMaxRetryDelayDuration());
+ assertEquals(Duration.ofMinutes(50), settings.getTotalTimeoutDuration());
+ assertEquals(Duration.ofSeconds(50),
settings.getInitialRpcTimeoutDuration());
+ assertEquals(1.0, settings.getRpcTimeoutMultiplier());
+ assertEquals(Duration.ofSeconds(50), settings.getMaxRpcTimeoutDuration());
+ }
+
+ @Test
+ void configuredValuesAreApplied() {
+ GoogleCloudConfig config = new GoogleCloudConfig();
+ config.setMaxAttempts("100");
+ config.setInitialRetryDelay("3");
+ config.setRetryDelayMultiplier("1.5");
+ config.setMaxRetryDelay("64");
+ config.setTotalTimeout("10");
+ config.setInitialRpcTimeout("20");
+ config.setRpcTimeoutMultiplier("2.0");
+ config.setMaxRpcTimeout("120");
+
+ RetrySettings settings =
GoogleStorageFileSystem.buildRetrySettings(config);
+
+ assertEquals(100, settings.getMaxAttempts());
+ assertEquals(Duration.ofSeconds(3),
settings.getInitialRetryDelayDuration());
+ assertEquals(1.5, settings.getRetryDelayMultiplier());
+ assertEquals(Duration.ofSeconds(64), settings.getMaxRetryDelayDuration());
+ assertEquals(Duration.ofSeconds(20),
settings.getInitialRpcTimeoutDuration());
+ assertEquals(2.0, settings.getRpcTimeoutMultiplier());
+ assertEquals(Duration.ofSeconds(120), settings.getMaxRpcTimeoutDuration());
+ }
+
+ /** Total timeout is in minutes while everything around it is in seconds. */
+ @Test
+ void totalTimeoutIsInterpretedAsMinutes() {
+ GoogleCloudConfig config = new GoogleCloudConfig();
+ config.setTotalTimeout("7");
+
+ assertEquals(
+ Duration.ofMinutes(7),
+
GoogleStorageFileSystem.buildRetrySettings(config).getTotalTimeoutDuration());
+ }
+
+ /**
+ * The max RPC timeout used to be left at the library default of 50s while
the initial one was
+ * applied, so raising the initial timeout past 50 threw an
IllegalStateException out of the
+ * client builder and took down all GCS access.
+ */
+ @Test
+ void initialRpcTimeoutAboveTheConfiguredMaximumDoesNotBreakTheClient() {
+ GoogleCloudConfig config = new GoogleCloudConfig();
+ config.setInitialRpcTimeout("1000");
+ config.setMaxRpcTimeout("50");
+
+ RetrySettings settings =
+ assertDoesNotThrow(() ->
GoogleStorageFileSystem.buildRetrySettings(config));
+
+ assertEquals(Duration.ofSeconds(1000),
settings.getInitialRpcTimeoutDuration());
+ assertEquals(
+ Duration.ofSeconds(1000),
+ settings.getMaxRpcTimeoutDuration(),
+ "the ceiling should be raised to the initial timeout rather than
rejected");
+ }
+
+ @Test
+ void maxRpcTimeoutReachesTheSettings() {
+ GoogleCloudConfig config = new GoogleCloudConfig();
+ config.setMaxRpcTimeout("300");
+
+ assertEquals(
+ Duration.ofSeconds(300),
+
GoogleStorageFileSystem.buildRetrySettings(config).getMaxRpcTimeoutDuration(),
+ "the max RPC timeout field was stored but never applied");
+ }
+
+ /** A cleared field in the dialog used to throw a NumberFormatException on
every GCS call. */
+ @Test
+ void blankAndInvalidValuesFallBackToTheDefaultInsteadOfThrowing() {
+ GoogleCloudConfig config = new GoogleCloudConfig();
+ config.setMaxAttempts("");
+ config.setInitialRetryDelay(null);
+ config.setRetryDelayMultiplier("not-a-number");
+ config.setMaxRetryDelay("");
+ config.setTotalTimeout("nope");
+ config.setInitialRpcTimeout("");
+ config.setRpcTimeoutMultiplier("");
+ config.setMaxRpcTimeout(null);
+
+ RetrySettings settings =
+ assertDoesNotThrow(() ->
GoogleStorageFileSystem.buildRetrySettings(config));
+
+ assertEquals(6, settings.getMaxAttempts());
+ assertEquals(Duration.ofSeconds(1),
settings.getInitialRetryDelayDuration());
+ assertEquals(2.0, settings.getRetryDelayMultiplier());
+ assertEquals(Duration.ofMinutes(50), settings.getTotalTimeoutDuration());
+ }
+
+ @Test
+ void retryStrategyFollowsTheNonIdempotentOption() {
+ GoogleCloudConfig config = new GoogleCloudConfig();
+
+ assertSame(
+ StorageRetryStrategy.getDefaultStorageRetryStrategy(),
+ GoogleStorageFileSystem.selectRetryStrategy(config),
+ "writes should not be retried unless the option is switched on");
+
+ config.setRetryNonIdempotentOperations(true);
+ // The uniform strategy is handed out as a new instance per call, so
compare by type.
+ assertEquals(
+ StorageRetryStrategy.getUniformStorageRetryStrategy().getClass(),
+ GoogleStorageFileSystem.selectRetryStrategy(config).getClass());
+ }
+}
