throw error when an upload fails due to bad state

TileDB-Inc · Mar 13, 2024 · a820cdb · a820cdb
1 parent 607d5ad
commit a820cdb
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 17 deletions.
diff --git a/tiledb/sm/filesystem/s3.cc b/tiledb/sm/filesystem/s3.cc
@@ -53,6 +53,7 @@
 #include <aws/s3/model/CreateMultipartUploadRequest.h>
 #include <aws/sts/STSClient.h>
 #include <boost/interprocess/streams/bufferstream.hpp>
+#include <cstdlib>
 #include <fstream>
 #include <iostream>
 
@@ -633,13 +634,11 @@ Status S3::disconnect() {
             Aws::S3::Model::AbortMultipartUploadRequest abort_request =
                 make_multipart_abort_request(*state);
             auto outcome = client_->AbortMultipartUpload(abort_request);
-            if (!outcome.IsSuccess()) {
-              const Status st = LOG_STATUS(Status_S3Error(
-                  std::string("Failed to disconnect and flush S3 objects. ") +
-                  outcome_error_message(outcome)));
-              if (!st.ok()) {
-                ret_st = st;
-              }
+            const Status st = LOG_STATUS(Status_S3Error(
+                std::string("Failed to disconnect and flush S3 objects. ") +
+                outcome_error_message(outcome)));
+            if (!st.ok()) {
+              ret_st = st;
             }
           }
           return Status::Ok();
@@ -714,7 +713,7 @@ Status S3::flush_object(const URI& uri) {
 
     throw_if_not_ok(wait_for_object_to_propagate(move(bucket), move(key)));
 
-    return finish_flush_object(std::move(outcome), uri, buff);
+    return finish_flush_object(std::move(outcome), uri, buff, false);
   } else {
     Aws::S3::Model::AbortMultipartUploadRequest abort_request =
         make_multipart_abort_request(*state);
@@ -723,7 +722,7 @@ Status S3::flush_object(const URI& uri) {
 
     state_lck.unlock();
 
-    return finish_flush_object(std::move(outcome), uri, buff);
+    return finish_flush_object(std::move(outcome), uri, buff, true);
   }
 }
 
@@ -799,11 +798,9 @@ void S3::finalize_and_flush_object(const URI& uri) {
         make_multipart_abort_request(state);
 
     auto outcome = client_->AbortMultipartUpload(abort_request);
-    if (!outcome.IsSuccess()) {
-      throw S3Exception{
-          std::string("Failed to flush S3 object ") + uri.c_str() +
-          outcome_error_message(outcome)};
-    }
+    throw S3Exception{
+        std::string("Failed to flush S3 object ") + uri.c_str() +
+        outcome_error_message(outcome)};
   }
 
   // Remove intermediate chunk files if any
@@ -1689,7 +1686,8 @@ template <typename R, typename E>
 Status S3::finish_flush_object(
     const Aws::Utils::Outcome<R, E>& outcome,
     const URI& uri,
-    Buffer* const buff) {
+    Buffer* const buff,
+    bool is_abort) {
   Aws::Http::URI aws_uri = uri.c_str();
 
   UniqueWriteLock unique_wl(&multipart_upload_rwlock_);
@@ -1701,7 +1699,7 @@ Status S3::finish_flush_object(
   file_buffers_lck.unlock();
   tdb_delete(buff);
 
-  if (!outcome.IsSuccess()) {
+  if (!outcome.IsSuccess() || is_abort) {
     return LOG_STATUS(Status_S3Error(
         std::string("Failed to flush S3 object ") + uri.c_str() +
         outcome_error_message(outcome)));

diff --git a/tiledb/sm/filesystem/s3.h b/tiledb/sm/filesystem/s3.h
@@ -1509,13 +1509,15 @@ class S3 : FilesystemBase {
    * @param outcome The returned outcome from the complete or abort request.
    * @param uri The URI of the S3 file to be written to.
    * @param buff The file buffer associated with 'uri'.
+   * @param abort Should be true only when this is an abort request.
    * @return Status
    */
   template <typename R, typename E>
   Status finish_flush_object(
       const Aws::Utils::Outcome<R, E>& outcome,
       const URI& uri,
-      Buffer* const buff);
+      Buffer* const buff,
+      bool is_abort);
 
   /**
    * Writes the input buffer to a file by issuing one PutObject