address comments

facebookincubator · May 14, 2024 · 8eedc3d · 8eedc3d
1 parent 404658a
commit 8eedc3d
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 10 deletions.
diff --git a/velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp b/velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp
@@ -654,9 +654,11 @@ class S3FileSystem::Impl {
     if (retryMode.has_value()) {
       if (retryMode.value() == "standard") {
         if (maxAttempts.has_value()) {
-          VELOX_USER_CHECK(
-              (maxAttempts.value() > 0),
-              "Invalid configuration: specify 'hive.s3.max-attempts' > 0.");
+          VELOX_USER_CHECK_GE(
+              maxAttempts.value(),
+              0,
+              "Invalid configuration: specified 'hive.s3.max-attempts' value {} is < 0.",
+              maxAttempts.value());
           return std::make_shared<Aws::Client::StandardRetryStrategy>(
               maxAttempts.value());
         } else {
@@ -665,9 +667,11 @@ class S3FileSystem::Impl {
         }
       } else if (retryMode.value() == "adaptive") {
         if (maxAttempts.has_value()) {
-          VELOX_USER_CHECK(
-              (maxAttempts.value() > 0),
-              "Invalid configuration: specify 'hive.s3.max-attempts' > 0.");
+          VELOX_USER_CHECK_GE(
+              maxAttempts.value(),
+              0,
+              "Invalid configuration: specified 'hive.s3.max-attempts' value {} is < 0.",
+              maxAttempts.value());
           return std::make_shared<Aws::Client::AdaptiveRetryStrategy>(
               maxAttempts.value());
         } else {
@@ -676,9 +680,11 @@ class S3FileSystem::Impl {
         }
       } else if (retryMode.value() == "legacy") {
         if (maxAttempts.has_value()) {
-          VELOX_USER_CHECK(
-              (maxAttempts.value() > 0),
-              "Invalid configuration: specify 'hive.s3.max-attempts' > 0.");
+          VELOX_USER_CHECK_GE(
+              maxAttempts.value(),
+              0,
+              "Invalid configuration: specified 'hive.s3.max-attempts' value {} is < 0.",
+              maxAttempts.value());
           return std::make_shared<Aws::Client::DefaultRetryStrategy>(
               maxAttempts.value());
         } else {

diff --git a/velox/connectors/hive/storage_adapters/s3fs/S3Util.h b/velox/connectors/hive/storage_adapters/s3fs/S3Util.h
@@ -173,7 +173,7 @@ inline std::string getRequestID(
           getRequestID(error.GetResponseHeaders()));                                                                            \
       if (IsRetryableHttpResponseCode(error.GetResponseCode())) {                                                               \
         auto retryHint = fmt::format(                                                                                           \
-            " This request gets retriable response and has retried {} times, you may increase 'hive.s3.max-attempts'.",         \
+            " Request failed after retrying {} times. Try increasing the value of 'hive.s3.max-attempts'.",                     \
             outcome.GetRetryCount());                                                                                           \
         errMsg.append(retryHint);                                                                                               \
       }                                                                                                                         \