update

Author: ywangd

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

@@ -38,6 +38,13 @@
 
 import static java.util.Collections.emptyMap;
 
+/**
+ * The S3ClientsManager is responsible for managing Amazon S3 clients associated with either the cluster or projects.
+ * To use a single data structure for all clients, the cluster clients are stored against the default project-id.
+ * Note that the cluster level clients are created and refreshed based on the ReloadablePlugin interface, while
+ * the project level clients are created and refreshed by cluster state updates. All clients are released when
+ * the manager itself is closed.
+ */
 public class S3ClientsManager implements ClusterStateApplier {
 
     private static final Logger logger = LogManager.getLogger(S3ClientsManager.class);
@@ -188,8 +195,8 @@ AmazonS3Reference client(ProjectId projectId, RepositoryMetadata repositoryMetad
     }
 
     /**
-     * Similar to S3Service#releaseCachedClients but only clears the cache for the given project.
-     * All clients for the project are closed and will be recreated on next access, also similar to S3Service#releaseCachedClients
+     * Clears the cache for the given project (default project-id is for the cluster level clients).
+     * All clients for the project are closed and will be recreated on next access.
      */
     void releaseCachedClients(ProjectId projectId) {
         final var old = clientsHolders.get(Objects.requireNonNull(projectId));
@@ -202,7 +209,6 @@ void releaseCachedClients(ProjectId projectId) {
 
     /**
      * Shutdown the manager by closing all clients holders. This is called when the node is shutting down.
-     * It attempts to wait (1 min) for any async client closing to complete.
      */
     void close() {
         IOUtils.closeWhileHandlingException(clientsHolders.values());

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

@@ -169,14 +169,19 @@ public AmazonS3Reference client(RepositoryMetadata repositoryMetadata) {
     }
 
     /**
-     * Attempts to retrieve a project client from the project client manager. Throws if project-id or the client name does not exist.
-     * THe client maybe initialized lazily.
-     * Delegates to {@link #client(RepositoryMetadata)} when either of the followings is true:
-     * 1. Per-project client is disabled
-     * 2. Blobstore is cluster level (projectId = null)
+     * Attempts to retrieve either a cluster or project client from the client manager. Throws if project-id or
+     * the client name does not exist. The client maybe initialized lazily.
+     * @param projectId The project associated with the client, or null if the client is cluster level
      */
     public AmazonS3Reference client(@Nullable ProjectId projectId, RepositoryMetadata repositoryMetadata) {
-        return s3ClientsManager.client(projectId == null ? ProjectId.DEFAULT : projectId, repositoryMetadata);
+        return s3ClientsManager.client(effectiveProjectId(projectId), repositoryMetadata);
+    }
+
+    /**
+     * We use the default project-id for cluster level clients.
+     */
+    ProjectId effectiveProjectId(@Nullable ProjectId projectId) {
+        return projectId == null ? ProjectId.DEFAULT : projectId;
     }
 
     // visible for tests
@@ -418,13 +423,10 @@ public void onBlobStoreClose() {
     }
 
     /**
-     * Release all project clients.
-     * Delegates to {@link #onBlobStoreClose()} when either of the followings is true:
-     * 1. Per-project client is disabled
-     * 2. Blobstore is cluster level (projectId = null)
+     * Release clients for the specified project.
      */
     public void onBlobStoreClose(@Nullable ProjectId projectId) {
-        s3ClientsManager.releaseCachedClients(projectId == null ? ProjectId.DEFAULT : projectId);
+        s3ClientsManager.releaseCachedClients(effectiveProjectId(projectId));
     }
 
     @Override

modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientsManagerTests.java

@@ -295,8 +295,7 @@ public void testProjectClientsDisabled() {
             TestProjectResolvers.DEFAULT_PROJECT_ONLY,
             mock(ResourceWatcherService.class),
             () -> Region.of("es-test-region")
-        ) {
-        };
+        );
         s3ServiceWithNoProjectSupport.refreshAndClearCache(S3ClientSettings.load(clusterService.getSettings()));
         s3ServiceWithNoProjectSupport.start();
         assertNotNull(s3ServiceWithNoProjectSupport.getS3PerProjectClientManager());
@@ -317,6 +316,7 @@ public void testProjectClientsDisabled() {
 
     private Map<ProjectId, S3ClientsManager.ClientsHolder<?>> getClientsHoldersExcludeDefaultProject() {
         final var holders = s3ClientsManager.getClientsHolders();
+        // Clients holder for the default project always exists
         assertThat(holders, hasKey(ProjectId.DEFAULT));
         return holders.entrySet()
             .stream()