Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[KYUUBI #6367] Flink SQL engine supports RenewDelegationToken #6371

Closed
wants to merge 4 commits into from
Closed

[KYUUBI #6367] Flink SQL engine supports RenewDelegationToken #6371

wants to merge 4 commits into from

Conversation

wForget
Copy link
Member

@wForget wForget commented May 8, 2024 •
edited

🔍 Description

Issue References 🔗

This pull request fixes #6367

Describe Your Solution 🔧

  • Implement RenewDelegationToken method in FlinkTBinaryFrontendService.
  • Pass kyuubi.engine.credentials configuration when starting flink engine.

Types of changes 🔖

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

test connection:

"jdbc:hive2://hadoop-master1.orb.local:10009/default;hive.server2.proxy.user=spark;principal=kyuubi/_HOST@TEST.ORG?kyuubi.engine.type=FLINK_SQL;flink.execution.target=yarn-application"

flink engine builder command:

image

jobmanager log:

2024-05-22 07:46:46,545 INFO  org.apache.kyuubi.engine.flink.FlinkTBinaryFrontendService   [] - Add new unknown token Kind: HIVE_DELEGATION_TOKEN, Service: , Ident: 00 05 73 70 61 72 6b 04 68 69 76 65 28 6b 79 75 75 62 69 2f 68 61 64 6f 6f 70 2d 6d 61 73 74 65 72 31 2e 6f 72 62 2e 6c 6f 63 61 6c 40 54 45 53 54 2e 4f 52 47 8a 01 8f 9f 3f d5 4c 8a 01 8f c3 4c 59 4c 0b 06
2024-05-22 07:46:46,547 WARN  org.apache.kyuubi.engine.flink.FlinkTBinaryFrontendService   [] - Ignore token with earlier issue date: Kind: HDFS_DELEGATION_TOKEN, Service: 172.20.0.5:8020, Ident: (token for spark: HDFS_DELEGATION_TOKEN owner=spark, renewer=spark, realUser=kyuubi/hadoop-master1.orb.local@TEST.ORG, issueDate=1716363711750, maxDate=1716968511750, sequenceNumber=15, masterKeyId=7)
2024-05-22 07:46:46,548 INFO  org.apache.kyuubi.engine.flink.FlinkTBinaryFrontendService   [] - Update delegation tokens. The number of tokens sent by the server is 2. The actual number of updated tokens is 1.

Related Unit Tests

Checklist 📝

Be nice. Be informative.

@wForget wForget self-assigned this May 8, 2024
@codecov-commenter
Copy link

codecov-commenter commented May 8, 2024 •
edited

Codecov Report

Attention: Patch coverage is 85.71429% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 58.32%. Comparing base (9075fbb) to head (83b402a).
Report is 29 commits behind head on master.

Files Patch % Lines
...ala/org/apache/kyuubi/util/KyuubiHadoopUtils.scala 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##             master    #6371      +/-   ##
============================================
- Coverage     58.48%   58.32%   -0.16%     
  Complexity       24       24              
============================================
  Files           653      656       +3     
  Lines         39880    40262     +382     
  Branches       5481     5499      +18     
============================================
+ Hits          23322    23484     +162     
- Misses        14067    14273     +206     
- Partials       2491     2505      +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pan3793
pan3793 reviewed May 11, 2024
View reviewed changes
Copy link
Member

@pan3793 pan3793 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code change lgtm. but this is not covered by CI, please test locally and paste your testing result to the PR description

@pan3793 pan3793 added this to the v1.10.0 milestone May 11, 2024
wForget
wForget commented May 22, 2024
View reviewed changes
kyuubi-common/src/main/scala/org/apache/kyuubi/util/KyuubiHadoopUtils.scala Outdated
@@ -60,11 +60,11 @@ object KyuubiHadoopUtils extends Logging {
val byteStream = new ByteArrayOutputStream
creds.writeTokenStorageToStream(new DataOutputStream(byteStream))

Base64.getMimeEncoder.encodeToString(byteStream.toByteArray)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newline cannot be parsed by flink command, so I changed it to Base64.getEncoder

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please change it in an independent PR. and I searched the codebase, seems there are other places call Base64.getMimeEncoder, should we change them together?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please change it in an independent PR. and I searched the codebase, seems there are other places call Base64.getMimeEncoder, should we change them together?

reverted and send a new pr #6408

@wForget
Copy link
Member Author

wForget commented May 22, 2024

code change lgtm. but this is not covered by CI, please test locally and paste your testing result to the PR description

I updated the test results to description, this change is as expected. @pan3793 PTAL, thanks.

@wForget wForget requested a review from pan3793 May 22, 2024 07:52
@pan3793 pan3793 changed the title [KYUUBI #6367] Support RenewDelegationToken for flink sql engine [KYUUBI #6367] Flink SQL engine support RenewDelegationToken May 24, 2024
@pan3793 pan3793 changed the title [KYUUBI #6367] Flink SQL engine support RenewDelegationToken [KYUUBI #6367] Flink SQL engine supports RenewDelegationToken May 24, 2024
@pan3793 pan3793 closed this in 4cbecdc May 24, 2024
@pan3793
Copy link
Member

pan3793 commented May 24, 2024

Thanks, merged to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pan3793 pan3793 pan3793 approved these changes

Assignees

@wForget wForget

Labels
module:common module:flink module:server module:spark
Projects
None yet
Milestone
v1.10.0
Development

Successfully merging this pull request may close these issues.

[FEATURE] Support RenewDelegationToken for flink sql engine
3 participants
@wForget @codecov-commenter @pan3793