Review update

apache · Jun 19, 2023 · 5624af2 · 5624af2
1 parent b45280b
commit 5624af2
Show file tree

Hide file tree

Showing 5 changed files with 8 additions and 8 deletions.
diff --git a/docker/test/integration/cluster/checkers/AwsChecker.py b/docker/test/integration/cluster/checkers/AwsChecker.py
@@ -39,7 +39,7 @@ def check_s3_server_object_hash(self, container_name: str, expected_file_hash: s
         if code != 0:
             return False
         file_hash = md5_output.split(' ')[0].strip()
-        return code == 0 and file_hash == expected_file_hash
+        return file_hash == expected_file_hash
 
     @retry_check()
     def check_s3_server_object_metadata(self, container_name, content_type="application/octet-stream", metadata=dict()):

diff --git a/extensions/aws/processors/PutS3Object.cpp b/extensions/aws/processors/PutS3Object.cpp
@@ -223,7 +223,7 @@ void PutS3Object::ageOffMultipartUploads(const CommonProperties &common_properti
   aws::s3::ListMultipartUploadsRequestParameters list_params(common_properties.credentials, *client_config_);
   list_params.setClientConfig(common_properties.proxy, common_properties.endpoint_override_url);
   list_params.bucket = common_properties.bucket;
-  list_params.upload_max_age = multipart_upload_max_age_threshold_;
+  list_params.age_off_limit = multipart_upload_max_age_threshold_;
   list_params.use_virtual_addressing = use_virtual_addressing_;
   auto aged_off_uploads_in_progress = s3_wrapper_.listMultipartUploads(list_params);
   if (!aged_off_uploads_in_progress) {

diff --git a/extensions/aws/s3/MultipartUploadStateStorage.cpp b/extensions/aws/s3/MultipartUploadStateStorage.cpp
@@ -30,7 +30,6 @@ MultipartUploadStateStorage::MultipartUploadStateStorage(const std::string& stat
     state_file_path_ = std::filesystem::path(state_directory) / std::string(state_id + "-s3-multipart-upload-state.properties");
     if (!std::filesystem::exists(state_file_path_)) {
       std::filesystem::create_directories(state_file_path_.parent_path());
-      // std::ofstream ofs(state_file_path_);
     } else {
       loadFile();
     }

diff --git a/extensions/aws/s3/S3Wrapper.cpp b/extensions/aws/s3/S3Wrapper.cpp
@@ -409,12 +409,13 @@ FetchObjectResult S3Wrapper::fillFetchObjectResult(const GetObjectRequestParamet
   return result;
 }
 
-void S3Wrapper::addListMultipartUploadResults(const Aws::Vector<Aws::S3::Model::MultipartUpload>& uploads, std::optional<std::chrono::milliseconds> max_upload_age,
+void S3Wrapper::addListMultipartUploadResults(const Aws::Vector<Aws::S3::Model::MultipartUpload>& uploads, std::optional<std::chrono::milliseconds> age_off_limit,
     std::vector<MultipartUpload>& filtered_uploads) {
   const auto now = Aws::Utils::DateTime::Now();
   for (const auto& upload : uploads) {
-    if (max_upload_age && now - upload.GetInitiated() <= *max_upload_age) {
-      logger_->log_debug("Multipart upload with key '%s' and upload id '%s' did not meet the age limit", upload.GetKey(), upload.GetUploadId());
+    // if age_off_limit is set only list the aged off uploads
+    if (age_off_limit && now - upload.GetInitiated() <= *age_off_limit) {
+      logger_->log_debug("Multipart upload with key '%s' and upload id '%s' has not aged off yet", upload.GetKey(), upload.GetUploadId());
       continue;
     }
 
@@ -437,7 +438,7 @@ std::optional<std::vector<MultipartUpload>> S3Wrapper::listMultipartUploads(cons
     }
     const auto& uploads = aws_result->GetUploads();
     logger_->log_debug("AWS S3 List operation returned %zu multipart uploads. This result is%s truncated.", uploads.size(), aws_result->GetIsTruncated() ? "" : " not");
-    addListMultipartUploadResults(uploads, params.upload_max_age, result);
+    addListMultipartUploadResults(uploads, params.age_off_limit, result);
     if (aws_result->GetIsTruncated()) {
       request.SetKeyMarker(aws_result->GetNextKeyMarker());
     }

diff --git a/extensions/aws/s3/S3Wrapper.h b/extensions/aws/s3/S3Wrapper.h
@@ -201,7 +201,7 @@ struct ListMultipartUploadsRequestParameters : public RequestParameters {
   ListMultipartUploadsRequestParameters(const Aws::Auth::AWSCredentials& creds, const Aws::Client::ClientConfiguration& config)
     : RequestParameters(creds, config) {}
   std::string bucket;
-  std::optional<std::chrono::milliseconds> upload_max_age;
+  std::optional<std::chrono::milliseconds> age_off_limit;  // if set, only list the aged off uploads
   bool use_virtual_addressing = true;
 };