You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2023/01/15 14:25:41 UTC

[GitHub] [spark] tedyu opened a new pull request, #39580: [MINOR] Throw Timeout exception when SASL Request Retry is enabled

tedyu opened a new pull request, #39580:
URL: https://github.com/apache/spark/pull/39580

   ### What changes were proposed in this pull request?
   This PR adds check of spark.shuffle.sasl.enableRetries config before throwing SaslTimeoutException.
   
   ### Why are the changes needed?
   `spark.shuffle.sasl.enableRetries` has default value of false. 
   In `shouldRetry` of `RetryingBlockTransferor`:
   ```
       boolean isSaslTimeout = enableSaslRetries && e instanceof SaslTimeoutException;
   ```
   Meaning, when `enableSaslRetries` is false, `isSaslTimeout` would be false as well.
   Hence we don't need to throw `SaslTimeoutException` if `enableSaslRetries` is false.
   
   ### Does this PR introduce _any_ user-facing change?
   No.
   
   ### How was this patch tested?
   Existing test suite.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] tedyu closed pull request #39580: [MINOR] [FOLLOW-UP] Throw Timeout exception when SASL Request Retry is enabled

Posted by GitBox <gi...@apache.org>.
tedyu closed pull request #39580: [MINOR] [FOLLOW-UP] Throw Timeout exception when SASL Request Retry is enabled
URL: https://github.com/apache/spark/pull/39580


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] tedyu commented on pull request #39580: [MINOR] [FOLLOW-UP] Throw Timeout exception when SASL Request Retry is enabled

Posted by GitBox <gi...@apache.org>.
tedyu commented on PR #39580:
URL: https://github.com/apache/spark/pull/39580#issuecomment-1383164639

   cc @mridulm 


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] mridulm commented on pull request #39580: [MINOR] [FOLLOW-UP] Throw Timeout exception when SASL Request Retry is enabled

Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #39580:
URL: https://github.com/apache/spark/pull/39580#issuecomment-1383309053

   Thanks for the interest @tedyu  !
   Please do feel free to ping me in case there are other improvements or bug fixes you contribute 


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] mridulm commented on pull request #39580: [MINOR] [FOLLOW-UP] Throw Timeout exception when SASL Request Retry is enabled

Posted by GitBox <gi...@apache.org>.
mridulm commented on PR #39580:
URL: https://github.com/apache/spark/pull/39580#issuecomment-1383201783

   The underlying layer does not concern itself with whether sasl timeout is enabled or not - that is for the invoker to decide on.
   If there is a timeout while waiting for sasl response, it is a sasl timeout.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org