[GitHub] [spark] dongjoon-hyun commented on a change in pull request #30366: [SPARK-33440][CORE] Separate the calculation for the next renewal date for each delegation token

[GitBox <gi...@apache.org>]

dongjoon-hyun commented on a change in pull request #30366:
URL: https://github.com/apache/spark/pull/30366#discussion_r523015210



##########
File path: core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala
##########
@@ -50,25 +51,34 @@ private[deploy] class HadoopFSDelegationTokenProvider
       val fileSystems = HadoopFSDelegationTokenProvider.hadoopFSsToAccess(sparkConf, hadoopConf)
       val fetchCreds = fetchDelegationTokens(getTokenRenewer(hadoopConf), fileSystems, creds)
 
-      // Get the token renewal interval if it is not set. It will only be called once.
-      if (tokenRenewalInterval == null) {
-        tokenRenewalInterval = getTokenRenewalInterval(hadoopConf, sparkConf, fileSystems)
+      // Get the token renewals interval if it is not set. It will only be called once.
+      if (tokenRenewalIntervals == null) {
+        tokenRenewalIntervals = getTokenRenewalIntervals(hadoopConf, sparkConf, fileSystems)
       }
 
+      // Construct the map for "token kind" to "issue date", so that we can calculate the
+      // next renewal date per token when interval is available.
+      val tokenAndIssueDates = fetchCreds.getAllTokens.asScala
+        .filter(_.decodeIdentifier().isInstanceOf[AbstractDelegationTokenIdentifier])
+        .map { token =>
+          val identifier = token
+            .decodeIdentifier()
+            .asInstanceOf[AbstractDelegationTokenIdentifier]
+          identifier.getKind -> identifier.getIssueDate
+        }.toMap
+
       // Get the time of next renewal.
-      val nextRenewalDate = tokenRenewalInterval.flatMap { interval =>
-        val nextRenewalDates = fetchCreds.getAllTokens.asScala
-          .filter(_.decodeIdentifier().isInstanceOf[AbstractDelegationTokenIdentifier])
-          .map { token =>
-            val identifier = token
-              .decodeIdentifier()
-              .asInstanceOf[AbstractDelegationTokenIdentifier]
-            identifier.getIssueDate + interval
-          }
-        if (nextRenewalDates.isEmpty) None else Some(nextRenewalDates.min)
+      val currentTime = System.currentTimeMillis()
+      logDebug(s"Calculating next renewal date per token. Current timestamp is $currentTime")
+      val nextRenewalDates = tokenRenewalIntervals.flatMap { case (kind, interval) =>
+        tokenAndIssueDates.get(kind).map { issueDate =>
+          val nextRenewalDateForToken = issueDate + interval
+          logDebug(s"Next renewal date is $nextRenewalDateForToken for token ${kind.toString}")
+          nextRenewalDateForToken
+        }.filterNot(_ < currentTime)

Review comment:
       Can we hit the situation where `_ < currentTime` for all tokens? Do we need to distinguish this case from the case `tokenRenewalIntervals` is empty?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org