Renames and refactoring for reloadable plugins (#30992)

plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2Service.java

@@ -22,12 +22,14 @@
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.unit.TimeValue;
+
+import java.io.Closeable;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.function.Function;
 
-interface AwsEc2Service {
+interface AwsEc2Service extends Closeable {
     Setting<Boolean> AUTO_ATTRIBUTE_SETTING = Setting.boolSetting("cloud.node.auto_attributes", false, Property.NodeScope);
 
     class HostType {
@@ -79,25 +81,20 @@ class HostType {
             key -> Setting.listSetting(key, Collections.emptyList(), Function.identity(), Property.NodeScope));
 
     /**
-     * Creates then caches an {@code AmazonEC2} client using the current client
-     * settings.
+     * Builds then caches an {@code AmazonEC2} client using the current client
+     * settings. Returns an {@code AmazonEc2Reference} wrapper which should be
+     * released as soon as it is not required anymore.
      */
     AmazonEc2Reference client();
 
     /**
-     * Updates settings for building the client. Future client requests will use the
-     * new settings. Implementations SHOULD drop the client cache to prevent reusing
-     * the client with old settings from cache.
+     * Updates the settings for building the client and releases the cached one.
+     * Future client requests will use the new settings to lazily built the new
+     * client.
      *
-     * @param clientSettings
-     *            the new settings
-     * @return the old settings
+     * @param clientSettings the new refreshed settings
+     * @return the old stale settings
      */
-    Ec2ClientSettings updateClientSettings(Ec2ClientSettings clientSettings);
+    Ec2ClientSettings refreshAndClearCache(Ec2ClientSettings clientSettings);
 
-    /**
-     * Releases the cached client. Subsequent client requests will recreate the
-     * client instance. Does not touch the client settings.
-     */
-    void releaseCachedClient();
 }

plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImpl.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.discovery.ec2;
 
+import java.io.IOException;
 import java.util.Random;
 
 import com.amazonaws.ClientConfiguration;
@@ -127,12 +128,12 @@ public AmazonEc2Reference client() {
 
 
     /**
-     * Reloads the settings for the AmazonEC2 client. New clients will be build
-     * using these. Old client is usable until released. On release it will be
-     * destroyed instead of being returned to the cache.
+     * Refreshes the settings for the AmazonEC2 client. New clients will be build
+     * using these new settings. Old client is usable until released. On release it
+     * will be destroyed instead of being returned to the cache.
      */
     @Override
-    public synchronized Ec2ClientSettings updateClientSettings(Ec2ClientSettings clientSettings) {
+    public synchronized Ec2ClientSettings refreshAndClearCache(Ec2ClientSettings clientSettings) {
         // shutdown all unused clients
         // others will shutdown on their respective release
         releaseCachedClient();
@@ -142,7 +143,11 @@ public synchronized Ec2ClientSettings updateClientSettings(Ec2ClientSettings cli
     }
 
     @Override
-    public synchronized void releaseCachedClient() {
+    public void close() {
+        releaseCachedClient();
+    }
+
+    private synchronized void releaseCachedClient() {
         if (this.clientReference == null) {
             return;
         }
@@ -154,4 +159,5 @@ public synchronized void releaseCachedClient() {
         // it will be restarted on new client usage
         IdleConnectionReaper.shutdown();
     }
+
 }

plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryPlugin.java

@@ -31,7 +31,7 @@
 import org.elasticsearch.node.Node;
 import org.elasticsearch.plugins.DiscoveryPlugin;
 import org.elasticsearch.plugins.Plugin;
-import org.elasticsearch.plugins.ReInitializablePlugin;
+import org.elasticsearch.plugins.ReloadablePlugin;
 import org.elasticsearch.transport.TransportService;
 
 import java.io.BufferedReader;
@@ -50,7 +50,7 @@
 import java.util.Map;
 import java.util.function.Supplier;
 
-public class Ec2DiscoveryPlugin extends Plugin implements DiscoveryPlugin, ReInitializablePlugin {
+public class Ec2DiscoveryPlugin extends Plugin implements DiscoveryPlugin, ReloadablePlugin {
 
     private static Logger logger = Loggers.getLogger(Ec2DiscoveryPlugin.class);
     public static final String EC2 = "ec2";
@@ -85,7 +85,7 @@ protected Ec2DiscoveryPlugin(Settings settings, AwsEc2ServiceImpl ec2Service) {
         this.settings = settings;
         this.ec2Service = ec2Service;
         // eagerly load client settings when secure settings are accessible
-        reinit(settings);
+        reload(settings);
     }
 
     @Override
@@ -172,14 +172,13 @@ static Settings getAvailabilityZoneNodeAttributes(Settings settings, String azMe
 
     @Override
     public void close() throws IOException {
-        ec2Service.releaseCachedClient();
+        ec2Service.close();
     }
 
     @Override
-    public boolean reinit(Settings settings) {
+    public void reload(Settings settings) {
         // secure settings should be readable
         final Ec2ClientSettings clientSettings = Ec2ClientSettings.getClientSettings(settings);
-        ec2Service.updateClientSettings(clientSettings);
-        return true;
+        ec2Service.refreshAndClearCache(clientSettings);
     }
 }

plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryPluginTests.java

@@ -135,7 +135,7 @@ public void testClientSettingsReInit() throws IOException {
                 assertThat(((AmazonEc2Mock) clientReference.client()).configuration.getProxyPort(), is(881));
                 assertThat(((AmazonEc2Mock) clientReference.client()).endpoint, is("ec2_endpoint_1"));
                 // reload secure settings2
-                plugin.reinit(settings2);
+                plugin.reload(settings2);
                 // client is not released, it is still using the old settings
                 assertThat(((AmazonEc2Mock) clientReference.client()).credentials.getCredentials().getAWSAccessKeyId(), is("ec2_access_1"));
                 assertThat(((AmazonEc2Mock) clientReference.client()).credentials.getCredentials().getAWSSecretKey(), is("ec2_secret_1"));

plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java

@@ -54,9 +54,9 @@ public AzureBlobStore(RepositoryMetaData metadata, Settings settings, AzureStora
         this.service = service;
         // locationMode is set per repository, not per client
         this.locationMode = Repository.LOCATION_MODE_SETTING.get(metadata.settings());
-        final Map<String, AzureStorageSettings> prevSettings = this.service.updateClientsSettings(emptyMap());
+        final Map<String, AzureStorageSettings> prevSettings = this.service.refreshAndClearCache(emptyMap());
         final Map<String, AzureStorageSettings> newSettings = AzureStorageSettings.overrideLocationMode(prevSettings, this.locationMode);
-        this.service.updateClientsSettings(newSettings);
+        this.service.refreshAndClearCache(newSettings);
     }
 
     @Override

plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepositoryPlugin.java

@@ -24,7 +24,7 @@
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.plugins.Plugin;
-import org.elasticsearch.plugins.ReInitializablePlugin;
+import org.elasticsearch.plugins.ReloadablePlugin;
 import org.elasticsearch.plugins.RepositoryPlugin;
 import org.elasticsearch.repositories.Repository;
 import java.util.Arrays;
@@ -35,7 +35,7 @@
 /**
  * A plugin to add a repository type that writes to and from the Azure cloud storage service.
  */
-public class AzureRepositoryPlugin extends Plugin implements RepositoryPlugin, ReInitializablePlugin {
+public class AzureRepositoryPlugin extends Plugin implements RepositoryPlugin, ReloadablePlugin {
 
     // protected for testing
     final AzureStorageService azureStoreService;
@@ -65,10 +65,9 @@ public List<Setting<?>> getSettings() {
     }
 
     @Override
-    public boolean reinit(Settings settings) {
+    public void reload(Settings settings) {
         // secure settings should be readable
         final Map<String, AzureStorageSettings> clientsSettings = AzureStorageSettings.load(settings);
-        azureStoreService.updateClientsSettings(clientsSettings);
-        return true;
+        azureStoreService.refreshAndClearCache(clientsSettings);
     }
 }

plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageService.java

@@ -50,14 +50,13 @@ public interface AzureStorageService {
     Tuple<CloudBlobClient, Supplier<OperationContext>> client(String clientName);
 
     /**
-     * Updates settings for building clients. Future client requests will use the
-     * new settings.
+     * Updates settings for building clients. Any client cache is cleared. Future
+     * client requests will use the new refreshed settings.
      *
-     * @param clientsSettings
-     *            the new settings
+     * @param clientsSettings the settings for new clients
      * @return the old settings
      */
-    Map<String, AzureStorageSettings> updateClientsSettings(Map<String, AzureStorageSettings> clientsSettings);
+    Map<String, AzureStorageSettings> refreshAndClearCache(Map<String, AzureStorageSettings> clientsSettings);
 
     ByteSizeValue MIN_CHUNK_SIZE = new ByteSizeValue(1, ByteSizeUnit.BYTES);
     ByteSizeValue MAX_CHUNK_SIZE = new ByteSizeValue(64, ByteSizeUnit.MB);

plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageServiceImpl.java

@@ -61,7 +61,7 @@ public AzureStorageServiceImpl(Settings settings) {
         super(settings);
         // eagerly load client settings so that secure settings are read
         final Map<String, AzureStorageSettings> clientsSettings = AzureStorageSettings.load(settings);
-        updateClientsSettings(clientsSettings);
+        refreshAndClearCache(clientsSettings);
     }
 
     @Override
@@ -107,7 +107,7 @@ protected OperationContext buildOperationContext(AzureStorageSettings azureStora
     }
 
     @Override
-    public Map<String, AzureStorageSettings> updateClientsSettings(Map<String, AzureStorageSettings> clientsSettings) {
+    public Map<String, AzureStorageSettings> refreshAndClearCache(Map<String, AzureStorageSettings> clientsSettings) {
         final Map<String, AzureStorageSettings> prevSettings = this.storageSettings;
         this.storageSettings = MapBuilder.newMapBuilder(clientsSettings).immutableMap();
         // clients are built lazily by {@link client(String)}

plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageSettings.java

@@ -185,7 +185,7 @@ public String toString() {
     }
 
     /**
-     * Parses settings and read all settings available under azure.client.*
+     * Parse and read all settings available under the azure.client.* namespace
      * @param settings settings to parse
      * @return All the named configurations
      */

plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceMock.java

@@ -212,7 +212,7 @@ public Tuple<CloudBlobClient, Supplier<OperationContext>> client(String clientNa
     }
 
     @Override
-    public Map<String, AzureStorageSettings> updateClientsSettings(Map<String, AzureStorageSettings> clientsSettings) {
+    public Map<String, AzureStorageSettings> refreshAndClearCache(Map<String, AzureStorageSettings> clientsSettings) {
         return emptyMap();
     }
 }

plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceTests.java

@@ -95,7 +95,7 @@ public void testReinitClientSettings() throws IOException {
             final SettingsException e1 = expectThrows(SettingsException.class, () -> azureStorageService.client("azure3"));
             assertThat(e1.getMessage(), is("Unable to find client with name [azure3]"));
             // update client settings
-            plugin.reinit(settings2);
+            plugin.reload(settings2);
             // old client 1 not changed
             assertThat(client11.getEndpoint().toString(), equalTo("https://myaccount11.blob.core.windows.net"));
             // new client 1 is changed
@@ -122,7 +122,7 @@ public void testReinitClientEmptySettings() throws IOException {
             final CloudBlobClient client11 = azureStorageService.client("azure1").v1();
             assertThat(client11.getEndpoint().toString(), equalTo("https://myaccount1.blob.core.windows.net"));
             // reinit with empty settings
-            final SettingsException e = expectThrows(SettingsException.class, () -> plugin.reinit(Settings.EMPTY));
+            final SettingsException e = expectThrows(SettingsException.class, () -> plugin.reload(Settings.EMPTY));
             assertThat(e.getMessage(), is("If you want to use an azure repository, you need to define a client configuration."));
             // existing client untouched
             assertThat(client11.getEndpoint().toString(), equalTo("https://myaccount1.blob.core.windows.net"));
@@ -145,7 +145,7 @@ public void testReinitClientWrongSettings() throws IOException {
             final AzureStorageServiceImpl azureStorageService = (AzureStorageServiceImpl) plugin.azureStoreService;
             final CloudBlobClient client11 = azureStorageService.client("azure1").v1();
             assertThat(client11.getEndpoint().toString(), equalTo("https://myaccount1.blob.core.windows.net"));
-            plugin.reinit(settings2);
+            plugin.reload(settings2);
             // existing client untouched
             assertThat(client11.getEndpoint().toString(), equalTo("https://myaccount1.blob.core.windows.net"));
             final SettingsException e = expectThrows(SettingsException.class, () -> azureStorageService.client("azure1"));

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

@@ -24,23 +24,23 @@
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.plugins.Plugin;
-import org.elasticsearch.plugins.ReInitializablePlugin;
+import org.elasticsearch.plugins.ReloadablePlugin;
 import org.elasticsearch.plugins.RepositoryPlugin;
 import org.elasticsearch.repositories.Repository;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
-public class GoogleCloudStoragePlugin extends Plugin implements RepositoryPlugin, ReInitializablePlugin {
+public class GoogleCloudStoragePlugin extends Plugin implements RepositoryPlugin, ReloadablePlugin {
 
     // package-private for tests
     final GoogleCloudStorageService storageService;
 
     public GoogleCloudStoragePlugin(final Settings settings) {
         this.storageService = createStorageService(settings);
         // eagerly load client settings so that secure settings are readable (not closed)
-        reinit(settings);
+        reload(settings);
     }
 
     // overridable for tests
@@ -67,14 +67,13 @@ public List<Setting<?>> getSettings() {
     }
 
     @Override
-    public boolean reinit(Settings settings) {
+    public void reload(Settings settings) {
         // Secure settings should be readable inside this method. Duplicate client
         // settings in a format (`GoogleCloudStorageClientSettings`) that does not
         // require for the `SecureSettings` to be open. Pass that around (the
         // `GoogleCloudStorageClientSettings` instance) instead of the `Settings`
         // instance.
         final Map<String, GoogleCloudStorageClientSettings> clientsSettings = GoogleCloudStorageClientSettings.load(settings);
-        this.storageService.updateClientsSettings(clientsSettings);
-        return true;
+        this.storageService.refreshAndClearCache(clientsSettings);
     }
 }

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

@@ -55,15 +55,15 @@ public GoogleCloudStorageService(final Settings settings) {
     }
 
     /**
-     * Updates the client settings and clears the client cache. Subsequent calls to
+     * Refreshes the client settings and clears the client cache. Subsequent calls to
      * {@code GoogleCloudStorageService#client} will return new clients constructed
      * using these passed settings.
      *
      * @param clientsSettings the new settings used for building clients for subsequent requests
      * @return previous settings which have been substituted
      */
     public synchronized Map<String, GoogleCloudStorageClientSettings>
-            updateClientsSettings(Map<String, GoogleCloudStorageClientSettings> clientsSettings) {
+            refreshAndClearCache(Map<String, GoogleCloudStorageClientSettings> clientsSettings) {
         final Map<String, GoogleCloudStorageClientSettings> prevSettings = this.clientsSettings;
         this.clientsSettings = MapBuilder.newMapBuilder(clientsSettings).immutableMap();
         this.clientsCache = emptyMap();

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

@@ -64,7 +64,7 @@ public void testClientInitializer() throws Exception {
                 .put(GoogleCloudStorageClientSettings.PROJECT_ID_SETTING.getConcreteSettingForNamespace(clientName).getKey(), projectIdName)
                 .build();
         final GoogleCloudStorageService service = new GoogleCloudStorageService(settings);
-        service.updateClientsSettings(GoogleCloudStorageClientSettings.load(settings));
+        service.refreshAndClearCache(GoogleCloudStorageClientSettings.load(settings));
         final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> service.client("another_client"));
         assertThat(e.getMessage(), Matchers.startsWith("Unknown client name"));
         assertSettingDeprecationsAndWarnings(
@@ -100,7 +100,7 @@ public void testReinitClientSettings() throws Exception {
             final IllegalArgumentException e1 = expectThrows(IllegalArgumentException.class, () -> storageService.client("gcs3"));
             assertThat(e1.getMessage(), containsString("Unknown client name [gcs3]."));
             // update client settings
-            plugin.reinit(settings2);
+            plugin.reload(settings2);
             // old client 1 not changed
             assertThat(client11.getOptions().getProjectId(), equalTo("project_gcs11"));
             // new client 1 is changed

plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/AwsS3Service.java

@@ -19,30 +19,25 @@
 
 package org.elasticsearch.repositories.s3;
 
+import java.io.Closeable;
 import java.util.Map;
 
-interface AwsS3Service {
+interface AwsS3Service extends Closeable {
 
     /**
      * Creates then caches an {@code AmazonS3} client using the current client
-     * settings.
+     * settings. Returns an {@code AmazonS3Reference} wrapper which has to be
+     * released as soon as it is not needed anymore.
      */
     AmazonS3Reference client(String clientName);
 
     /**
-     * Updates settings for building clients. Future client requests will use the
-     * new settings. Implementations SHOULD drop the client cache to prevent reusing
-     * clients with old settings from cache.
+     * Updates settings for building clients and clears the client cache. Future
+     * client requests will use the new settings to lazily build new clients.
      *
-     * @param clientsSettings
-     *            the new settings
-     * @return the old settings
+     * @param clientsSettings the new refreshed settings
+     * @return the old stale settings
      */
-    Map<String, S3ClientSettings> updateClientsSettings(Map<String, S3ClientSettings> clientsSettings);
+    Map<String, S3ClientSettings> refreshAndClearCache(Map<String, S3ClientSettings> clientsSettings);
 
-    /**
-     * Releases cached clients. Subsequent client requests will recreate client
-     * instances. Does not touch the client settings.
-     */
-    void releaseCachedClients();
 }