You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/08/12 10:18:46 UTC

[GitHub] [incubator-kyuubi] zhouyifan279 opened a new pull request, #3225: [KYUUBI #3219] Error renew delegation tokens: Unknown version of delegation token 8

zhouyifan279 opened a new pull request, #3225:
URL: https://github.com/apache/incubator-kyuubi/pull/3225

   ### _Why are the changes needed?_
   Spark SQL Engine failed to get issue date from some DelegationTokenIdentifiers, such as OzoneTokenIdentifier.
   
   
   ### _How was this patch tested?_
   - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
   
   - [ ] Add screenshots for manual tests if appropriate
   
   - [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
   


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org


[GitHub] [incubator-kyuubi] pan3793 commented on pull request #3225: [KYUUBI #3219] Error renew delegation tokens: Unknown version of delegation token 8

Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3225:
URL: https://github.com/apache/incubator-kyuubi/pull/3225#issuecomment-1214563284

   Thanks, merging to master


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #3225: [KYUUBI #3219] Error renew delegation tokens: Unknown version of delegation token 8

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3225:
URL: https://github.com/apache/incubator-kyuubi/pull/3225#discussion_r945393535


##########
kyuubi-common/src/main/scala/org/apache/kyuubi/util/KyuubiHadoopUtils.scala:
##########
@@ -85,13 +87,23 @@ object KyuubiHadoopUtils {
       .toMap
   }
 
-  def getTokenIssueDate(token: Token[_ <: TokenIdentifier]): Long = {
-    // It is safe to deserialize any token identifier to hdfs `DelegationTokenIdentifier`
-    // as all token identifiers have the same binary format.
-    val tokenIdentifier = new DelegationTokenIdentifier
-    val buf = new ByteArrayInputStream(token.getIdentifier)
-    val in = new DataInputStream(buf)
-    tokenIdentifier.readFields(in)
-    tokenIdentifier.getIssueDate
+  def getTokenIssueDate(token: Token[_ <: TokenIdentifier]): Option[Long] = {
+    token.decodeIdentifier() match {
+      case tokenIdent: AbstractDelegationTokenIdentifier =>
+        Some(tokenIdent.getIssueDate)
+      case null =>
+        // TokenIdentifiers not found in ServiceLoader
+        val tokenIdentifier = new DelegationTokenIdentifier
+        val buf = new ByteArrayInputStream(token.getIdentifier)
+        val in = new DataInputStream(buf)
+        Try(tokenIdentifier.readFields(in)) match {
+          case Success(_) =>
+            Some(tokenIdentifier.getIssueDate)
+          case Failure(_) =>
+            None
+        }
+      case _ =>
+        None
+    }

Review Comment:
   For debug purposes, I suggest add some clues in negative case



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org


[GitHub] [incubator-kyuubi] zhouyifan279 commented on pull request #3225: [KYUUBI #3219] Error renew delegation tokens: Unknown version of delegation token 8

Posted by GitBox <gi...@apache.org>.
zhouyifan279 commented on PR #3225:
URL: https://github.com/apache/incubator-kyuubi/pull/3225#issuecomment-1213125845

   cc @yaooqinn @MLikeWater 


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org


[GitHub] [incubator-kyuubi] pan3793 closed pull request #3225: [KYUUBI #3219] Error renew delegation tokens: Unknown version of delegation token 8

Posted by GitBox <gi...@apache.org>.
pan3793 closed pull request #3225: [KYUUBI #3219] Error renew delegation tokens: Unknown version of delegation token 8
URL: https://github.com/apache/incubator-kyuubi/pull/3225


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org


[GitHub] [incubator-kyuubi] zhouyifan279 commented on a diff in pull request #3225: [KYUUBI #3219] Error renew delegation tokens: Unknown version of delegation token 8

Posted by GitBox <gi...@apache.org>.
zhouyifan279 commented on code in PR #3225:
URL: https://github.com/apache/incubator-kyuubi/pull/3225#discussion_r945395360


##########
kyuubi-common/src/main/scala/org/apache/kyuubi/util/KyuubiHadoopUtils.scala:
##########
@@ -85,13 +87,23 @@ object KyuubiHadoopUtils {
       .toMap
   }
 
-  def getTokenIssueDate(token: Token[_ <: TokenIdentifier]): Long = {
-    // It is safe to deserialize any token identifier to hdfs `DelegationTokenIdentifier`
-    // as all token identifiers have the same binary format.
-    val tokenIdentifier = new DelegationTokenIdentifier
-    val buf = new ByteArrayInputStream(token.getIdentifier)
-    val in = new DataInputStream(buf)
-    tokenIdentifier.readFields(in)
-    tokenIdentifier.getIssueDate
+  def getTokenIssueDate(token: Token[_ <: TokenIdentifier]): Option[Long] = {
+    token.decodeIdentifier() match {
+      case tokenIdent: AbstractDelegationTokenIdentifier =>
+        Some(tokenIdent.getIssueDate)
+      case null =>
+        // TokenIdentifiers not found in ServiceLoader
+        val tokenIdentifier = new DelegationTokenIdentifier
+        val buf = new ByteArrayInputStream(token.getIdentifier)
+        val in = new DataInputStream(buf)
+        Try(tokenIdentifier.readFields(in)) match {
+          case Success(_) =>
+            Some(tokenIdentifier.getIssueDate)
+          case Failure(_) =>
+            None
+        }
+      case _ =>
+        None
+    }

Review Comment:
   Done



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org


[GitHub] [incubator-kyuubi] zhouyifan279 commented on pull request #3225: [KYUUBI #3219] Error renew delegation tokens: Unknown version of delegation token 8

Posted by GitBox <gi...@apache.org>.
zhouyifan279 commented on PR #3225:
URL: https://github.com/apache/incubator-kyuubi/pull/3225#issuecomment-1214542662

   Also cc @pan3793 


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org


[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #3225: [KYUUBI #3219] Error renew delegation tokens: Unknown version of delegation token 8

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #3225:
URL: https://github.com/apache/incubator-kyuubi/pull/3225#issuecomment-1214558745

   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3225](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c5a37a0) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/e847ab3566f8cbc840fed2216b22ee7c12655b39?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e847ab3) will **decrease** coverage by `0.05%`.
   > The diff coverage is `90.00%`.
   
   > :exclamation: Current head c5a37a0 differs from pull request most recent head 0ffdcf9. Consider uploading reports for the commit 0ffdcf9 to get more accurate results
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #3225      +/-   ##
   ============================================
   - Coverage     51.68%   51.63%   -0.06%     
     Complexity        6        6              
   ============================================
     Files           464      464              
     Lines         25651    25662      +11     
     Branches       3551     3556       +5     
   ============================================
   - Hits          13259    13251       -8     
   - Misses        11133    11141       +8     
   - Partials       1259     1270      +11     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ubi/engine/spark/SparkTBinaryFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9TcGFya1RCaW5hcnlGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `82.27% <85.71%> (-0.62%)` | :arrow_down: |
   | [...ala/org/apache/kyuubi/util/KyuubiHadoopUtils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS91dGlsL0t5dXViaUhhZG9vcFV0aWxzLnNjYWxh) | `94.87% <92.30%> (-1.91%)` | :arrow_down: |
   | [.../kyuubi/server/mysql/constant/MySQLErrorCode.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvY29uc3RhbnQvTXlTUUxFcnJvckNvZGUuc2NhbGE=) | `13.84% <0.00%> (-6.16%)` | :arrow_down: |
   | [...ache/kyuubi/server/mysql/MySQLCommandHandler.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvTXlTUUxDb21tYW5kSGFuZGxlci5zY2FsYQ==) | `76.13% <0.00%> (-3.41%)` | :arrow_down: |
   | [...ache/kyuubi/server/mysql/MySQLGenericPackets.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvTXlTUUxHZW5lcmljUGFja2V0cy5zY2FsYQ==) | `76.59% <0.00%> (-2.13%)` | :arrow_down: |
   | [...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xvZy9Mb2dCYXRjaENvbW1hbmQuc2NhbGE=) | `78.00% <0.00%> (-2.00%)` | :arrow_down: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uTWFuYWdlci5zY2FsYQ==) | `80.82% <0.00%> (-1.37%)` | :arrow_down: |
   | [...mon/src/main/scala/org/apache/kyuubi/Logging.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9Mb2dnaW5nLnNjYWxh) | `43.42% <0.00%> (-1.32%)` | :arrow_down: |
   | [.../engine/spark/session/SparkSQLSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9zZXNzaW9uL1NwYXJrU1FMU2Vzc2lvbk1hbmFnZXIuc2NhbGE=) | `78.48% <0.00%> (-1.27%)` | :arrow_down: |
   | [...ain/scala/org/apache/kyuubi/engine/EngineRef.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvRW5naW5lUmVmLnNjYWxh) | `74.07% <0.00%> (-0.93%)` | :arrow_down: |
   | ... and [3 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/3225/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org