tweak

Author: ywangd

modules/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GcsPerProjectClientManager.java

@@ -25,7 +25,7 @@ public class GcsPerProjectClientManager implements ClusterStateListener {
 
     private final Settings settings;
     private final BiFunction<GoogleCloudStorageClientSettings, GcsRepositoryStatsCollector, MeteredStorage> clientBuilder;
-    private final Map<ProjectId, ClientHolder> perProjectClientsCache;
+    private final Map<ProjectId, ClientsHolder> perProjectClientsCache;
 
     public GcsPerProjectClientManager(
         Settings settings,
@@ -40,7 +40,7 @@ public GcsPerProjectClientManager(
     public void clusterChanged(ClusterChangedEvent event) {
         final Map<ProjectId, ProjectMetadata> currentProjects = event.state().metadata().projects();
 
-        final var updatedPerProjectClients = new HashMap<ProjectId, ClientHolder>();
+        final var updatedPerProjectClients = new HashMap<ProjectId, ClientsHolder>();
         for (var project : currentProjects.values()) {
             final ProjectSecrets projectSecrets = project.custom(ProjectSecrets.TYPE);
             if (projectSecrets == null) {
@@ -61,9 +61,10 @@ public void clusterChanged(ClusterChangedEvent event) {
             // TODO: Building and comparing the whole GoogleCloudStorageClientSettings may be insufficient, we could just compare the
             // relevant secrets
             if (newOrUpdated(project.id(), clientSettings)) {
-                updatedPerProjectClients.put(project.id(), new ClientHolder(clientSettings));
+                updatedPerProjectClients.put(project.id(), new ClientsHolder(clientSettings));
             }
         }
+
         // Updated projects
         perProjectClientsCache.putAll(updatedPerProjectClients);
 
@@ -81,39 +82,39 @@ public MeteredStorage client(
         String repositoryName,
         GcsRepositoryStatsCollector statsCollector
     ) {
-        final var clientHolder = perProjectClientsCache.get(projectId);
-        if (clientHolder == null) {
+        final var clientsHolder = perProjectClientsCache.get(projectId);
+        if (clientsHolder == null) {
             throw new IllegalArgumentException("No project found for [" + projectId + "]");
         }
-        return clientHolder.client(clientName, repositoryName, statsCollector);
+        return clientsHolder.client(clientName, repositoryName, statsCollector);
     }
 
     public void closeRepositoryClients(ProjectId projectId, String repositoryName) {
-        final var clientHolder = perProjectClientsCache.get(projectId);
-        if (clientHolder != null) {
-            clientHolder.closeRepositoryClients(repositoryName);
+        final var clientsHolder = perProjectClientsCache.get(projectId);
+        if (clientsHolder != null) {
+            clientsHolder.closeRepositoryClients(repositoryName);
         }
     }
 
     private boolean newOrUpdated(ProjectId projectId, Map<String, GoogleCloudStorageClientSettings> currentClientSettings) {
-        if (perProjectClientsCache.containsKey(projectId) == false) {
+        final ClientsHolder old = perProjectClientsCache.get(projectId);
+        if (old == null) {
             return true;
         }
-        final var previousClientSettings = perProjectClientsCache.get(projectId).clientSettings();
-        return currentClientSettings.equals(previousClientSettings) == false;
+        return currentClientSettings.equals(old.clientSettings()) == false;
     }
 
-    private final class ClientHolder {
+    private final class ClientsHolder {
         // clientName -> client settings
         private final Map<String, GoogleCloudStorageClientSettings> clientSettings;
         // repositoryName -> client
         private final Map<String, MeteredStorage> clientCache = new ConcurrentHashMap<>();
 
-        ClientHolder(Map<String, GoogleCloudStorageClientSettings> clientSettings) {
+        ClientsHolder(Map<String, GoogleCloudStorageClientSettings> clientSettings) {
             this.clientSettings = clientSettings;
         }
 
-        public Map<String, GoogleCloudStorageClientSettings> clientSettings() {
+        Map<String, GoogleCloudStorageClientSettings> clientSettings() {
             return clientSettings;
         }
 

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3PerProjectClientManager.java

@@ -32,10 +32,8 @@
 
 public class S3PerProjectClientManager implements ClusterStateListener {
 
-    // Original settings at node startup time
     private final Settings settings;
     private final Function<S3ClientSettings, AmazonS3> clientBuilder;
-
     // A map of projectId to clients holder. Adding to and removing from the map happen only with the cluster state listener thread.
     private final Map<ProjectId, ClientsHolder> perProjectClientsCache;
 
@@ -107,11 +105,11 @@ public void clearCacheForProject(ProjectId projectId) {
         final var old = perProjectClientsCache.get(projectId);
         if (old != null) {
             old.clearCache();
+            // TODO: do we need this?
+            // shutdown IdleConnectionReaper background thread
+            // it will be restarted on new client usage
+            IdleConnectionReaper.shutdown();
         }
-        // TODO: do we need this?
-        // shutdown IdleConnectionReaper background thread
-        // it will be restarted on new client usage
-        IdleConnectionReaper.shutdown();
     }
 
     /**
@@ -126,8 +124,7 @@ private boolean newOrUpdated(ProjectId projectId, Map<String, S3ClientSettings>
         if (old == null) {
             return true;
         }
-        final var oldClientSettings = old.clientSettings();
-        return currentClientSettings.equals(oldClientSettings) == false;
+        return currentClientSettings.equals(old.clientSettings()) == false;
     }
 
     private final class ClientsHolder implements Closeable {
@@ -139,11 +136,11 @@ private final class ClientsHolder implements Closeable {
             this.clientSettings = clientSettings;
         }
 
-        public Map<String, S3ClientSettings> clientSettings() {
+        Map<String, S3ClientSettings> clientSettings() {
             return clientSettings;
         }
 
-        public AmazonS3Reference client(String clientName) {
+        AmazonS3Reference client(String clientName) {
             final var clientReference = clientsCache.get(clientName);
             // It is ok to retrieve an existing client when the cache is being cleared or the holder is closing.
             // As long as there are paired incRef/decRef calls, the client will be closed when the last reference is released
@@ -174,10 +171,10 @@ public AmazonS3Reference client(String clientName) {
         }
 
         /**
-         * Clear the cache by closing and clear out all clients. New {@link #client(String)} call will recreate
+         * Clear the cache by closing and clear out all clients. Subsequent {@link #client(String)} calls will recreate
          * the clients and populate the cache again.
          */
-        public synchronized void clearCache() {
+        synchronized void clearCache() {
             IOUtils.closeWhileHandlingException(clientsCache.values());
             clientsCache = Collections.emptyMap();
         }