juliuszsompolski commented on code in PR #43757:
URL: https://github.com/apache/spark/pull/43757#discussion_r1389568029
##########
connector/connect/common/src/main/scala/org/apache/spark/sql/connect/client/SparkConnectStubState.scala:
##########
@@ -28,15 +28,15 @@ import org.apache.spark.internal.Logging
// logic.
class SparkConnectStubState(
channel: ManagedChannel,
- val retryPolicy: GrpcRetryHandler.RetryPolicy)
+ val retryHandler: GrpcRetryHandler)
extends Logging {
+ def this(channel: ManagedChannel, retryPolicies:
Seq[GrpcRetryHandler.RetryPolicy]) =
+ this(channel, new GrpcRetryHandler(retryPolicies))
+
// Responsible to convert the GRPC Status exceptions into Spark exceptions.
lazy val exceptionConverter: GrpcExceptionConverter = new
GrpcExceptionConverter(channel)
- // Manages the retry handler logic used by the stubs.
- lazy val retryHandler = new GrpcRetryHandler(retryPolicy)
-
Review Comment:
Reverting these changes, there is still one retryHandler shared across
everyone who wants it.
There is only one SparkConnectStubState per client (that's why it got
refactored out of BlockingStub, and Stub - so that all the stubs etc. and every
use of it within a client share it). So it's only one retryHandler either way.
If you have multiple clients, they have multiple retryHandlers anyway.
The only place (outside testing) that creates it is in SparkConnectClient,
uses this constructor anyway:
```
private[this] val stubState = new SparkConnectStubState(channel,
configuration.retryPolicy)
```
so the only change that changes in this files do is:
* make retryHandler a `val` instead of `lazy val` - since the other objects
in this state are lazy, it would be more consistent if it stayed lazy.
* have two constructors, and the new default one passing the
GrpcRetryHandler is unused - why have an unused constructor?
--
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]
