[GitHub] [spark] attilapiros commented on a change in pull request #24160: [SPARK-27219][core] Treat timeouts as fatal in SASL fallback path.

[GitBox <gi...@apache.org>]

attilapiros commented on a change in pull request #24160: [SPARK-27219][core] Treat timeouts as fatal in SASL fallback path.
URL: https://github.com/apache/spark/pull/24160#discussion_r267840721
 
 

 ##########
 File path: common/network-common/src/main/java/org/apache/spark/network/crypto/AuthClientBootstrap.java
 ##########
 @@ -82,13 +83,19 @@ public void doBootstrap(TransportClient client, Channel channel) {
     } catch (RuntimeException e) {
       // There isn't a good exception that can be caught here to know whether it's really
       // OK to switch back to SASL (because the server doesn't speak the new protocol). So
-      // try it anyway, and in the worst case things will fail again.
-      if (conf.saslFallback()) {
-        LOG.warn("New auth protocol failed, trying SASL.", e);
-        doSaslAuth(client, channel);
-      } else {
+      // try it anyway, unless it's a timeout, which is locally fatal. In the worst case
+      // things will fail again.
+      if (!conf.saslFallback() || e.getCause() instanceof TimeoutException) {
         throw e;
       }
+
+      if (!LOG.isDebugEnabled()) {
 
 Review comment:
   Super NIT: avoid  the `!` operator by swapping the if and the else body.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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