HwangDongJun commented on code in PR #58187:
URL: https://github.com/apache/spark/pull/58187#discussion_r4163609340
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala:
##########
@@ -94,12 +113,45 @@ private[spark] object ClientArguments {
* implemented features.
* @param kubernetesClient the client to talk to the Kubernetes API server
* @param watcher a watcher that monitors and logs the application status
+ * @param recoveryClientFactoryOverride overrides how the SPARK-38079
shutdown-hook cleanup path
+ * (see run()) builds the fresh
Kubernetes client it uses to
+ * best-effort delete pre-resources left
orphaned by an
+ * abrupt termination. Not reused from
`kubernetesClient`
+ * because that one may already be
closed, or concurrently
+ * in use, by the time the hook runs.
Exposed only for
+ * testing; `None` (the default) builds a
real client the
+ * same way
KubernetesClientApplication.run() builds
+ * `kubernetesClient`. A plain default
value can't itself
+ * call `conf.sparkConf` here, since
`conf` is a
+ * constructor parameter, not yet a class
member, at the
+ * point default values are evaluated.
+ * @param shutdownHookOpsOverride overrides how the SPARK-38079 cleanup hook
(see run()) is
+ * registered/removed. Exposed only for
testing -- actually
+ * triggering a JVM shutdown hook from a test
is impractical, so
+ * tests instead inject fakes here to assert
that run() registers
+ * a hook before applying pre-resources and
removes that exact
+ * hook once it is done with them (success or
failure). `None`
+ * (the default) delegates to the real
`ShutdownHookManager`.
*/
private[spark] class Client(
conf: KubernetesDriverConf,
builder: KubernetesDriverBuilder,
kubernetesClient: KubernetesClient,
- watcher: LoggingPodStatusWatcher) extends Logging {
+ watcher: LoggingPodStatusWatcher,
+ recoveryClientFactoryOverride: Option[() => KubernetesClient] = None,
+ shutdownHookOpsOverride: Option[ShutdownHookOps] = None) extends Logging {
+
+ private val recoveryClientFactory: () => KubernetesClient =
recoveryClientFactoryOverride
Review Comment:
This whole mechanism is gone -- the duplicated client creation, the
`recoveryClientFactoryOverride`/`shutdownHookOpsOverride` constructor
parameters, `ShutdownHookOps` and its Scaladoc, all removed along with the
shutdown hook itself (replaced by `spec.schedulingGates`, see [the reply
above](https://github.com/apache/spark/pull/58187#discussion_r4129910563)).
There's nothing left that builds a second client or re-runs kubeconfig
auto-configuration during shutdown -- `Client.run()` only ever uses the one
`kubernetesClient` passed in, for its entire sequential run.
--
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]
