You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "yma11 (via GitHub)" <gi...@apache.org> on 2024/05/08 09:16:11 UTC
[PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
yma11 opened a new pull request, #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660
## What changes were proposed in this pull request?
Add more configs for AWS s3
(Fixes: \#5659)
## How was this patch tested?
CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "yma11 (via GitHub)" <gi...@apache.org>.
yma11 commented on code in PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#discussion_r1643819612
##########
cpp/velox/utils/ConfigExtractor.cc:
##########
@@ -34,6 +34,12 @@ const bool kVeloxFileHandleCacheEnabledDefault = false;
// Log granularity of AWS C++ SDK
const std::string kVeloxAwsSdkLogLevel = "spark.gluten.velox.awsSdkLogLevel";
const std::string kVeloxAwsSdkLogLevelDefault = "FATAL";
+// Retry mode for AWS s3
+const std::string kVeloxS3RetryMode = "spark.gluten.velox.fs.s3a.retry.mode";
+const std::string kVeloxS3RetryModeDefault = "legacy";
+// Connection timeout for AWS s3
+const std::string kVeloxS3ConnectTimeout = "spark.gluten.velox.fs.s3a.connect.timeout";
+const std::string kVeloxS3ConnectTimeoutDefault = "1s";
Review Comment:
`200s` seems quite a large number. @FelixYBW what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#issuecomment-2172494439
Run Gluten Clickhouse CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#issuecomment-2100132624
https://github.com/apache/incubator-gluten/issues/5659
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#issuecomment-2177338251
Run Gluten Clickhouse CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "leesf (via GitHub)" <gi...@apache.org>.
leesf commented on code in PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#discussion_r1642807814
##########
shims/common/src/main/scala/org/apache/gluten/GlutenConfig.scala:
##########
@@ -687,6 +699,10 @@ object GlutenConfig {
(SPARK_S3_USE_INSTANCE_CREDENTIALS, "false"),
(SPARK_S3_IAM, ""),
(SPARK_S3_IAM_SESSION_NAME, ""),
+ (SPARK_S3_RETRY_MAX_ATTEMPTS, "3"),
+ (SPARK_S3_CONNECTION_MAXIMUM, "96"),
Review Comment:
`96`/`3` can also be the default value for the config and use something like `AWS_S3_CONNECT_TIMEOUT.defaultValueString` as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW commented on PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#issuecomment-2176480447
Is the Velox PR merged? can this PR be merged?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#issuecomment-2100133118
Run Gluten Clickhouse CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "leesf (via GitHub)" <gi...@apache.org>.
leesf commented on code in PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#discussion_r1642803514
##########
shims/common/src/main/scala/org/apache/gluten/GlutenConfig.scala:
##########
@@ -1927,6 +1943,20 @@ object GlutenConfig {
.stringConf
.createWithDefault("FATAL")
+ val AWS_S3_RETRY_MODE =
+ buildConf("spark.gluten.velox.fs.s3a.retry.mode")
+ .internal()
+ .doc("Retry mode for AWS s3 connection error.")
Review Comment:
Can add `"standard" and "adaptive"` as well for the doc?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "yma11 (via GitHub)" <gi...@apache.org>.
yma11 commented on code in PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#discussion_r1645272377
##########
shims/common/src/main/scala/org/apache/gluten/GlutenConfig.scala:
##########
@@ -687,6 +699,10 @@ object GlutenConfig {
(SPARK_S3_USE_INSTANCE_CREDENTIALS, "false"),
(SPARK_S3_IAM, ""),
(SPARK_S3_IAM_SESSION_NAME, ""),
+ (SPARK_S3_RETRY_MAX_ATTEMPTS, "3"),
+ (SPARK_S3_CONNECTION_MAXIMUM, "96"),
Review Comment:
Yes. These keys can also use `defaultValueString` but existing code also use the value directly for easy reading unless it's a new config in gluten itself. So let's keep it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "leesf (via GitHub)" <gi...@apache.org>.
leesf commented on PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#issuecomment-2177331474
> Is the Velox PR merged? can this PR be merged?
Yes, related changes have been merged in velox. https://github.com/facebookincubator/velox/commit/74fe2ba145efbf927fcdb6215d91a6899513a2c2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "leesf (via GitHub)" <gi...@apache.org>.
leesf commented on code in PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#discussion_r1642815943
##########
cpp/velox/utils/ConfigExtractor.cc:
##########
@@ -34,6 +34,12 @@ const bool kVeloxFileHandleCacheEnabledDefault = false;
// Log granularity of AWS C++ SDK
const std::string kVeloxAwsSdkLogLevel = "spark.gluten.velox.awsSdkLogLevel";
const std::string kVeloxAwsSdkLogLevelDefault = "FATAL";
+// Retry mode for AWS s3
+const std::string kVeloxS3RetryMode = "spark.gluten.velox.fs.s3a.retry.mode";
+const std::string kVeloxS3RetryModeDefault = "legacy";
+// Connection timeout for AWS s3
+const std::string kVeloxS3ConnectTimeout = "spark.gluten.velox.fs.s3a.connect.timeout";
+const std::string kVeloxS3ConnectTimeoutDefault = "1s";
Review Comment:
can we align the default config value with the hadoop config. https://hadoop.apache.org/docs/r3.0.0/hadoop-project-dist/hadoop-common/core-default.xml , such as the default value for `fs.s3a.connection.timeout` is 200000ms
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "yma11 (via GitHub)" <gi...@apache.org>.
yma11 commented on PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#issuecomment-2177345421
@leesf Thanks for comments. Updated and please help review again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#issuecomment-2172498467
Run Gluten Clickhouse CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-5659][VL] Add more configs for AWS s3 [incubator-gluten]
Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW commented on code in PR #5660:
URL: https://github.com/apache/incubator-gluten/pull/5660#discussion_r1644733291
##########
cpp/velox/utils/ConfigExtractor.cc:
##########
@@ -34,6 +34,12 @@ const bool kVeloxFileHandleCacheEnabledDefault = false;
// Log granularity of AWS C++ SDK
const std::string kVeloxAwsSdkLogLevel = "spark.gluten.velox.awsSdkLogLevel";
const std::string kVeloxAwsSdkLogLevelDefault = "FATAL";
+// Retry mode for AWS s3
+const std::string kVeloxS3RetryMode = "spark.gluten.velox.fs.s3a.retry.mode";
+const std::string kVeloxS3RetryModeDefault = "legacy";
+// Connection timeout for AWS s3
+const std::string kVeloxS3ConnectTimeout = "spark.gluten.velox.fs.s3a.connect.timeout";
+const std::string kVeloxS3ConnectTimeoutDefault = "1s";
Review Comment:
Let's use hadoop's default one
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org