Skip to content

Commit 0aafb06

Browse files
albertzaharovitsalbertzaharovits
committed
Fix the mishandled backport 873d380 (#30638)
Closes #30738
1 parent 75ecf58 commit 0aafb06

File tree

5 files changed

+91
-16
lines changed

5 files changed

+91
-16
lines changed

‎plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageRepository.java

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package org.elasticsearch.repositories.gcs;
2121

22+
import org.apache.logging.log4j.Logger;
2223
import org.elasticsearch.cluster.metadata.RepositoryMetaData;
2324
import org.elasticsearch.common.Strings;
2425
import org.elasticsearch.common.blobstore.BlobPath;
@@ -28,6 +29,7 @@
2829
import org.elasticsearch.common.settings.Setting;
2930
import org.elasticsearch.common.unit.ByteSizeUnit;
3031
import org.elasticsearch.common.unit.ByteSizeValue;
32+
import org.elasticsearch.common.unit.TimeValue;
3133
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
3234
import org.elasticsearch.env.Environment;
3335
import org.elasticsearch.repositories.RepositoryException;
@@ -46,12 +48,17 @@
4648

4749
class GoogleCloudStorageRepository extends BlobStoreRepository {
4850

51+
private final Logger logger = ESLoggerFactory.getLogger(GoogleCloudStorageRepository.class);
52+
private final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
53+
4954
// package private for testing
5055
static final ByteSizeValue MIN_CHUNK_SIZE = new ByteSizeValue(1, ByteSizeUnit.BYTES);
5156
static final ByteSizeValue MAX_CHUNK_SIZE = new ByteSizeValue(100, ByteSizeUnit.MB);
5257

5358
static final String TYPE = "gcs";
5459

60+
static final TimeValue NO_TIMEOUT = timeValueMillis(-1);
61+
5562
static final Setting<String> BUCKET =
5663
simpleString("bucket", Property.NodeScope, Property.Dynamic);
5764
static final Setting<String> BASE_PATH =
@@ -62,6 +69,18 @@ class GoogleCloudStorageRepository extends BlobStoreRepository {
6269
byteSizeSetting("chunk_size", MAX_CHUNK_SIZE, MIN_CHUNK_SIZE, MAX_CHUNK_SIZE, Property.NodeScope, Property.Dynamic);
6370
static final Setting<String> CLIENT_NAME = new Setting<>("client", "default", Function.identity());
6471

72+
@Deprecated
73+
static final Setting<String> APPLICATION_NAME =
74+
new Setting<>("application_name", "", Function.identity(), Property.NodeScope, Property.Dynamic);
75+
76+
@Deprecated
77+
static final Setting<TimeValue> HTTP_READ_TIMEOUT =
78+
timeSetting("http.read_timeout", NO_TIMEOUT, Property.NodeScope, Property.Dynamic);
79+
80+
@Deprecated
81+
static final Setting<TimeValue> HTTP_CONNECT_TIMEOUT =
82+
timeSetting("http.connect_timeout", NO_TIMEOUT, Property.NodeScope, Property.Dynamic);
83+
6584
private final ByteSizeValue chunkSize;
6685
private final boolean compress;
6786
private final BlobPath basePath;
@@ -90,7 +109,32 @@ class GoogleCloudStorageRepository extends BlobStoreRepository {
90109

91110
logger.debug("using bucket [{}], base_path [{}], chunk_size [{}], compress [{}]", bucket, basePath, chunkSize, compress);
92111

93-
Storage client = SocketAccess.doPrivilegedIOException(() -> storageService.createClient(clientName));
112+
String application = APPLICATION_NAME.get(metadata.settings());
113+
if (Strings.hasText(application)) {
114+
deprecationLogger.deprecated("Setting [application_name] in repository settings is deprecated, " +
115+
"it must be specified in the client settings instead");
116+
}
117+
TimeValue connectTimeout = null;
118+
TimeValue readTimeout = null;
119+
120+
TimeValue timeout = HTTP_CONNECT_TIMEOUT.get(metadata.settings());
121+
if ((timeout != null) && (timeout.millis() != NO_TIMEOUT.millis())) {
122+
deprecationLogger.deprecated("Setting [http.connect_timeout] in repository settings is deprecated, " +
123+
"it must be specified in the client settings instead");
124+
connectTimeout = timeout;
125+
}
126+
timeout = HTTP_READ_TIMEOUT.get(metadata.settings());
127+
if ((timeout != null) && (timeout.millis() != NO_TIMEOUT.millis())) {
128+
deprecationLogger.deprecated("Setting [http.read_timeout] in repository settings is deprecated, " +
129+
"it must be specified in the client settings instead");
130+
readTimeout = timeout;
131+
}
132+
133+
TimeValue finalConnectTimeout = connectTimeout;
134+
TimeValue finalReadTimeout = readTimeout;
135+
136+
Storage client = SocketAccess.doPrivilegedIOException(() ->
137+
storageService.createClient(clientName, application, finalConnectTimeout, finalReadTimeout));
94138
this.blobStore = new GoogleCloudStorageBlobStore(settings, bucket, client);
95139
}
96140

‎plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageService.java

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.google.cloud.storage.Storage;
2929
import com.google.cloud.storage.StorageOptions;
3030

31+
import org.elasticsearch.common.Nullable;
3132
import org.elasticsearch.common.Strings;
3233
import org.elasticsearch.common.collect.MapBuilder;
3334
import org.elasticsearch.common.component.AbstractComponent;
@@ -55,9 +56,15 @@ public GoogleCloudStorageService(final Environment environment, final Map<String
5556
* Creates a client that can be used to manage Google Cloud Storage objects.
5657
*
5758
* @param clientName name of client settings to use, including secure settings
59+
* @param application deprecated application name setting overriding client settings
60+
* @param connectTimeout deprecated connect timeout setting overriding client settings
61+
* @param readTimeout deprecated read timeout setting overriding client settings
5862
* @return a Client instance that can be used to manage Storage objects
5963
*/
60-
public Storage createClient(final String clientName) throws Exception {
64+
public Storage createClient(final String clientName,
65+
@Nullable final String application,
66+
@Nullable final TimeValue connectTimeout,
67+
@Nullable final TimeValue readTimeout) throws Exception {
6168

6269
final GoogleCloudStorageClientSettings clientSettings = clientsSettings.get(clientName);
6370
if (clientSettings == null) {
@@ -66,16 +73,17 @@ public Storage createClient(final String clientName) throws Exception {
6673
}
6774
final HttpTransport httpTransport = createHttpTransport(clientSettings.getHost());
6875
final HttpTransportOptions httpTransportOptions = HttpTransportOptions.newBuilder()
69-
.setConnectTimeout(toTimeout(clientSettings.getConnectTimeout()))
70-
.setReadTimeout(toTimeout(clientSettings.getReadTimeout()))
76+
.setConnectTimeout(connectTimeout != null ? toTimeout(connectTimeout) : toTimeout(clientSettings.getConnectTimeout()))
77+
.setReadTimeout(readTimeout != null ? toTimeout(readTimeout) : toTimeout(clientSettings.getReadTimeout()))
7178
.setHttpTransportFactory(() -> httpTransport)
7279
.build();
7380
final StorageOptions.Builder storageOptionsBuilder = StorageOptions.newBuilder()
7481
.setTransportOptions(httpTransportOptions)
7582
.setHeaderProvider(() -> {
7683
final MapBuilder<String, String> mapBuilder = MapBuilder.newMapBuilder();
77-
if (Strings.hasLength(clientSettings.getApplicationName())) {
78-
mapBuilder.put("user-agent", clientSettings.getApplicationName());
84+
final String applicationName = Strings.hasLength(application) ? application : clientSettings.getApplicationName();
85+
if (Strings.hasLength(applicationName)) {
86+
mapBuilder.put("user-agent", applicationName);
7987
}
8088
return mapBuilder.immutableMap();
8189
});

‎plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.common.settings.Settings;
2525
import org.elasticsearch.common.unit.ByteSizeUnit;
2626
import org.elasticsearch.common.unit.ByteSizeValue;
27+
import org.elasticsearch.common.unit.TimeValue;
2728
import org.elasticsearch.env.Environment;
2829
import org.elasticsearch.plugins.Plugin;
2930
import org.elasticsearch.repositories.blobstore.ESBlobStoreRepositoryIntegTestCase;
@@ -85,7 +86,10 @@ public static class MockGoogleCloudStorageService extends GoogleCloudStorageServ
8586
}
8687

8788
@Override
88-
public Storage createClient(final String clientName) {
89+
public Storage createClient(final String clientName,
90+
final String application,
91+
final TimeValue connectTimeout,
92+
final TimeValue readTimeout) {
8993
return new MockStorage(BUCKET, blobs);
9094
}
9195
}

‎plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageRepositoryDeprecationTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.google.cloud.storage.Storage;
2323
import org.elasticsearch.cluster.metadata.RepositoryMetaData;
2424
import org.elasticsearch.common.settings.Settings;
25+
import org.elasticsearch.common.unit.TimeValue;
2526
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
2627
import org.elasticsearch.env.Environment;
2728
import org.elasticsearch.env.TestEnvironment;
@@ -31,7 +32,6 @@
3132

3233
public class GoogleCloudStorageRepositoryDeprecationTests extends ESTestCase {
3334

34-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/30638")
3535
public void testDeprecatedSettings() throws Exception {
3636
final Settings repositorySettings = Settings.builder()
3737
.put("bucket", "test")
@@ -46,7 +46,7 @@ public void testDeprecatedSettings() throws Exception {
4646
new GoogleCloudStorageRepository(repositoryMetaData, environment, NamedXContentRegistry.EMPTY,
4747
new GoogleCloudStorageService(environment, GoogleCloudStorageClientSettings.load(Settings.EMPTY)) {
4848
@Override
49-
public Storage createClient(String clientName) throws Exception {
49+
public Storage createClient(String clientName, String application, TimeValue connect, TimeValue read) throws Exception {
5050
return new MockStorage("test", new ConcurrentHashMap<>());
5151
}
5252
});

‎plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,38 @@ public void testClientInitializer() throws Exception {
5959
final GoogleCloudStorageClientSettings clientSettings = GoogleCloudStorageClientSettings.getClientSettings(settings, clientName);
6060
final GoogleCloudStorageService service = new GoogleCloudStorageService(environment,
6161
Collections.singletonMap(clientName, clientSettings));
62-
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> service.createClient("another_client"));
62+
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
63+
() -> service.createClient("another_client", null, null, null));
6364
assertThat(e.getMessage(), Matchers.startsWith("Unknown client name"));
6465
assertSettingDeprecationsAndWarnings(
6566
new Setting<?>[] { GoogleCloudStorageClientSettings.APPLICATION_NAME_SETTING.getConcreteSettingForNamespace(clientName) });
66-
final Storage storage = service.createClient(clientName);
67-
assertThat(storage.getOptions().getApplicationName(), Matchers.containsString(applicationName));
67+
final String deprecatedApplicationName = randomBoolean() ? null : "deprecated_" + randomAlphaOfLength(4);
68+
final TimeValue deprecatedConnectTimeout = randomBoolean() ? null : TimeValue.timeValueNanos(randomIntBetween(0, 2000000));
69+
final TimeValue deprecatedReadTimeout = randomBoolean() ? null : TimeValue.timeValueNanos(randomIntBetween(0, 2000000));
70+
final Storage storage = service.createClient(clientName, deprecatedApplicationName, deprecatedConnectTimeout,
71+
deprecatedReadTimeout);
72+
if (deprecatedApplicationName != null) {
73+
assertThat(storage.getOptions().getApplicationName(), Matchers.containsString(deprecatedApplicationName));
74+
} else {
75+
assertThat(storage.getOptions().getApplicationName(), Matchers.containsString(applicationName));
76+
}
6877
assertThat(storage.getOptions().getHost(), Matchers.is(hostName));
6978
assertThat(storage.getOptions().getProjectId(), Matchers.is(projectIdName));
7079
assertThat(storage.getOptions().getTransportOptions(), Matchers.instanceOf(HttpTransportOptions.class));
71-
assertThat(((HttpTransportOptions) storage.getOptions().getTransportOptions()).getConnectTimeout(),
72-
Matchers.is((int) connectTimeValue.millis()));
73-
assertThat(((HttpTransportOptions) storage.getOptions().getTransportOptions()).getReadTimeout(),
74-
Matchers.is((int) readTimeValue.millis()));
80+
if (deprecatedConnectTimeout != null) {
81+
assertThat(((HttpTransportOptions) storage.getOptions().getTransportOptions()).getConnectTimeout(),
82+
Matchers.is((int) deprecatedConnectTimeout.millis()));
83+
} else {
84+
assertThat(((HttpTransportOptions) storage.getOptions().getTransportOptions()).getConnectTimeout(),
85+
Matchers.is((int) connectTimeValue.millis()));
86+
}
87+
if (deprecatedReadTimeout != null) {
88+
assertThat(((HttpTransportOptions) storage.getOptions().getTransportOptions()).getReadTimeout(),
89+
Matchers.is((int) deprecatedReadTimeout.millis()));
90+
} else {
91+
assertThat(((HttpTransportOptions) storage.getOptions().getTransportOptions()).getReadTimeout(),
92+
Matchers.is((int) readTimeValue.millis()));
93+
}
7594
assertThat(storage.getOptions().getCredentials(), Matchers.nullValue(Credentials.class));
7695
}
7796

0 commit comments

Comments
 (0)