Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/19272#discussion_r146436327
--- Diff:
resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackend.scala
---
@@ -213,6 +216,24 @@ private[spark] class
MesosCoarseGrainedSchedulerBackend(
sc.conf.getOption("spark.mesos.driver.frameworkId").map(_ + suffix)
)
+ // check that the credentials are defined, even though it's likely
that auth would have failed
+ // already if you've made it this far
+ if (principal != null && currentHadoopDelegationTokens.isDefined) {
+ logDebug(s"Principal found ($principal) starting token renewer")
+ // The renewal time is ignored when getting the initial delegation
tokens
+ // (CoarseGrainedSchedulerBackend.scala:getHadoopDelegationCreds),
so we get the renewal
+ // time here and schedule a thread to renew them.
+ val renewalTime =
--- End diff --
I still don't like this. You should not need to implement this separate
method of getting the renewal time just because the current code is throwing
out that information. Instead you should fix the code so that the information
is preserved.
`getHadoopDelegationCreds` is called in only one place, so my suggestion
would be to encapsulate initializing the token manager and getting the initial
set of tokens into a single method (instead of the current two).
Then in that method's implementation you can get the initial set of tokens,
initialize the renewer thread with the correct renewal period, and return the
data needed by the scheduler.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
