You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2021/10/21 21:54:35 UTC

[lucene-solr] branch branch_8x updated: SOLR-15702: Fix S3Repository to follow BackupRepository.createDirectory() API contract

This is an automated email from the ASF dual-hosted git repository.

houston pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new 033bc9d  SOLR-15702: Fix S3Repository to follow BackupRepository.createDirectory() API contract
033bc9d is described below

commit 033bc9dc5d425a0a774d93712cac190329061a31
Author: Houston Putman <ho...@apache.org>
AuthorDate: Thu Oct 21 15:02:30 2021 -0400

    SOLR-15702: Fix S3Repository to follow BackupRepository.createDirectory() API contract
---
 solr/CHANGES.txt                                   |  5 +--
 .../java/org/apache/solr/s3/S3StorageClient.java   | 40 ++++++++--------------
 .../apache/solr/core/backup/BackupFilePaths.java   | 15 ++------
 3 files changed, 21 insertions(+), 39 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index aabe4c3..a990591 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -17,8 +17,7 @@ New Features
 
 Improvements
 ---------------------
-
-* SOLR-15702: Stabilize S3 tests by using waiters after directory creation (Houston Putman)
+(No changes)
 
 Optimizations
 ---------------------
@@ -30,6 +29,8 @@ Bug Fixes
 
 * SOLR-15691: Admin UI raises yellow warning even when only case of hostnames differ (janhoy)
 
+* SOLR-15702: Fix S3Repository to follow BackupRepository.createDirectory() API contract (Houston Putman)
+
 Build
 ---------------------
 
diff --git a/solr/contrib/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java b/solr/contrib/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java
index 31ac598..e080ca0 100644
--- a/solr/contrib/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java
+++ b/solr/contrib/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java
@@ -23,7 +23,6 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.lang.invoke.MethodHandles;
 import java.net.URI;
-import java.time.Duration;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashSet;
@@ -39,7 +38,6 @@ import org.slf4j.LoggerFactory;
 import software.amazon.awssdk.awscore.exception.AwsServiceException;
 import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.core.exception.SdkException;
-import software.amazon.awssdk.core.retry.backoff.BackoffStrategy;
 import software.amazon.awssdk.core.sync.RequestBody;
 import software.amazon.awssdk.http.apache.ApacheHttpClient;
 import software.amazon.awssdk.http.apache.ProxyConfiguration;
@@ -130,36 +128,28 @@ public class S3StorageClient {
     return clientBuilder.build();
   }
 
-  /** Create a directory in S3. */
+  /** Create a directory in S3, if it does not already exist. */
   void createDirectory(String path) throws S3Exception {
     String sanitizedDirPath = sanitizedDirPath(path);
 
-    if (!parentDirectoryExist(sanitizedDirPath)) {
+    // Only create the directory if it does not already exist
+    if (!pathExists(sanitizedDirPath)) {
       createDirectory(getParentDirectory(sanitizedDirPath));
       // TODO see https://issues.apache.org/jira/browse/SOLR-15359
       //            throw new S3Exception("Parent directory doesn't exist, path=" + path);
-    }
 
-    try {
-      // Create empty object with content type header
-      PutObjectRequest putRequest =
-          PutObjectRequest.builder()
-              .bucket(bucketName)
-              .contentType(S3_DIR_CONTENT_TYPE)
-              .key(sanitizedDirPath)
-              .build();
-      s3Client.putObject(putRequest, RequestBody.empty());
-      // Wait until the object exists to continue
-      s3Client
-          .waiter()
-          .waitUntilObjectExists(
-              builder -> builder.bucket(bucketName).key(sanitizedDirPath),
-              config ->
-                  config
-                      .waitTimeout(Duration.ofSeconds(5))
-                      .backoffStrategy(BackoffStrategy.defaultStrategy()));
-    } catch (SdkClientException ase) {
-      throw handleAmazonException(ase);
+      try {
+        // Create empty object with content type header
+        PutObjectRequest putRequest =
+            PutObjectRequest.builder()
+                .bucket(bucketName)
+                .contentType(S3_DIR_CONTENT_TYPE)
+                .key(sanitizedDirPath)
+                .build();
+        s3Client.putObject(putRequest, RequestBody.empty());
+      } catch (SdkClientException ase) {
+        throw handleAmazonException(ase);
+      }
     }
   }
 
diff --git a/solr/core/src/java/org/apache/solr/core/backup/BackupFilePaths.java b/solr/core/src/java/org/apache/solr/core/backup/BackupFilePaths.java
index 43b27dc..d1a54dc 100644
--- a/solr/core/src/java/org/apache/solr/core/backup/BackupFilePaths.java
+++ b/solr/core/src/java/org/apache/solr/core/backup/BackupFilePaths.java
@@ -83,18 +83,9 @@ public class BackupFilePaths {
      * @throws IOException for issues encountered using repository to create directories
      */
     public void createIncrementalBackupFolders() throws IOException {
-        if (!repository.exists(backupLoc)) {
-            repository.createDirectory(backupLoc);
-        }
-        URI indexDir = getIndexDir();
-        if (!repository.exists(indexDir)) {
-            repository.createDirectory(indexDir);
-        }
-
-        URI shardBackupMetadataDir = getShardBackupMetadataDir();
-        if (!repository.exists(shardBackupMetadataDir)) {
-            repository.createDirectory(shardBackupMetadataDir);
-        }
+        repository.createDirectory(backupLoc);
+        repository.createDirectory(getIndexDir());
+        repository.createDirectory(getShardBackupMetadataDir());
     }
 
     /**