Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/14065#discussion_r73423485
--- Diff:
yarn/src/main/scala/org/apache/spark/deploy/yarn/security/CredentialUpdater.scala
---
@@ -41,16 +43,18 @@ private[spark] class ExecutorDelegationTokenUpdater(
SparkHadoopUtil.get.getConfBypassingFSCache(
hadoopConf, new Path(credentialsFile).toUri.getScheme)
- private val delegationTokenRenewer =
+ private val credentialUpdater =
Executors.newSingleThreadScheduledExecutor(
- ThreadUtils.namedThreadFactory("Delegation Token Refresh Thread"))
+ ThreadUtils.namedThreadFactory("Credential Refresh Thread"))
- // On the executor, this thread wakes up and picks up new tokens from
HDFS, if any.
- private val executorUpdaterRunnable =
+ // This thread wakes up and picks up new credentials from HDFS, if any.
+ private val credentialUpdaterRunnable =
new Runnable {
override def run(): Unit =
Utils.logUncaughtExceptions(updateCredentialsIfRequired())
}
+ @volatile private var timeOfNextUpdate =
sparkConf.get(CREDENTIALS_UPDATE_TIME)
--- End diff --
Not sure I understand.
`timeOfNextUpdate` is set in L73, and is used in L82. So it doesn't seem
like keeping this state across calls of that method is necessary.
I can see a case where, if the directory where credentials are expected to
be stored is empty, then the code skips L73. That seems to be used to
"bootstrap" the updater; the first invocation of `updateCredentialsIfRequired`
will schedule an update based on the renewal time set in the conf.
I find that behavior a little odd and would prefer if the "bootstrapping"
were more explicit, i.e., if there was code to explicitly schedule the first
`updateCredentialsIfRequired` with the value of `CREDENTIALS_UPDATE_TIME`, and
have the code treat an empty directory the same way as if the more recent file
does not match the expected (i.e. L76).
I know this code is not being added as part of your change, but could you
do that cleanup?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
