mridulm commented on code in PR #41785:
URL: https://github.com/apache/spark/pull/41785#discussion_r1252417257


##########
common/network-common/src/main/java/org/apache/spark/network/client/TransportClientFactory.java:
##########
@@ -245,12 +245,13 @@ TransportClient createClient(InetSocketAddress address)
     logger.debug("Creating new connection to {}", address);
 
     Bootstrap bootstrap = new Bootstrap();
+    int connCreateTimeout = conf.connectionCreationTimeoutMs();

Review Comment:
   This failure is in `createClient` (when introduced) - and will fail all 
invocations for that module - which will in turn fail the task/stage/job. This 
is similar in behavior to a configuration referenced in executor which fails in 
`checkValue`.
   The failure reason will inform the user about the invalid configuration.
   
   Do you have a scenario where an exception thrown in this codepath does not 
result in failure ?
   If not, let us revert this PR and simplify the code.



##########
common/network-common/src/main/java/org/apache/spark/network/client/TransportClientFactory.java:
##########
@@ -245,12 +245,13 @@ TransportClient createClient(InetSocketAddress address)
     logger.debug("Creating new connection to {}", address);
 
     Bootstrap bootstrap = new Bootstrap();
+    int connCreateTimeout = conf.connectionCreationTimeoutMs();

Review Comment:
   This failure is in `createClient` (when introduced) - and will fail all 
invocations for that module - which will in turn fail the task/stage/job. This 
is similar in behavior to a configuration referenced in executor which fails in 
`checkValue`.
   The failure reason will inform the user about the invalid configuration.
   
   Do you have a scenario where an exception thrown in this codepath does not 
result in failure ?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to