You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "Madhukar525722 (via GitHub)" <gi...@apache.org> on 2024/10/23 21:35:12 UTC
[PR] [KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop … [kyuubi]
Madhukar525722 opened a new pull request, #6779:
URL: https://github.com/apache/kyuubi/pull/6779
…groups
# :mag: Description
## Issue References 🔗
<!-- Append the issue number after #. If there is no issue for you to link create one or -->
<!-- If there are no issues to link, please provide details here. -->
This pull request fixes #6402
## Describe Your Solution 🔧
Currently, the group level engine is getting launched with the first user group itself. This change will allow user to pass in which group they want to launch the engine.
The flow of implementation is :
- Take input of preferred group
- If it is not defined, take the default route, pick the first group from the list
- If its defined
- User is not part of preferred group, raise ERROR
- User is part of preferred group,
- If group engine don’t exists, then create a engine with group name owned by first sessionUser
- If group engine exists, use the existing one
1. In this way, for every group there will be single engine, which can be re-used.
2. First user launching the engine is implemented to handle the scenarios where Hadoop groups are not the yarn users.
3. For security concerns in 2nd point, we can have fine grained access control on session level, using Apache Ranger.
## Types of changes :bookmark:
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [x] 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 Results🧪
1. When PREFERRED_GROUP was not defined, it used the first group from the list
Spark application name: kyuubi_GROUP_SPARK_SQL_Internet_Users_default_d2826b5b-3f1e-48a0-b42f-d248da914b7c
application ID: application_1728291907264_43966
Start time: 2024-10-23T20:48:52.415
User: madlnu
2. When valid PREFERRED_GROUP was defined
Spark application name: kyuubi_GROUP_SPARK_SQL_kyuubi_test_b_default_be9a16a8-be38-4ab6-bee9-1934f8556f18
application ID: application_1728291907264_43968
Start time: 2024-10-23T20:50:25.944
User: madlnu
3. When invalid PREFERRED_GROUP was defined
Error: org.apache.kyuubi.KyuubiSQLException: Error operating LaunchEngine: java.lang.IllegalArgumentException: User is not part of the preferred group: kyuubi_test_x
4. When any other user X tries to access the existing GROUP engine, it uses the same engine
Spark application name: kyuubi_GROUP_SPARK_SQL_Internet_Users_default_d2826b5b-3f1e-48a0-b42f-d248da914b7c
application ID: application_1728291907264_43966
Start time: 2024-10-23T20:48:52.415
User: madlnu
---
# Checklist 📝
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)
**Be nice. Be informative.**
--
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
Re: [PR] [KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop … [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6779:
URL: https://github.com/apache/kyuubi/pull/6779#issuecomment-2441077505
@Madhukar525722 thanks for taking care of this feature, I leave the comments to add a configuration "kyuubi.session.preferGroup" previously, but I have a little different idea now.
Now I would suggest having a "kyuubi.session.preferredGroups" (Seq[String]), when present, we select the most preferred group from the whole group list, otherwise, take the head, instead of failing fast. For the implementatio, we can use a custom `Comparator` to acheive that.
> And I have worries about using the session conf will interference choosing the engine reference, as it's easy to be changed at runtime. Is there any better approach with solid connection variables?
seems there is no much differences from the existing properties like `kyuubi.session.user`?
--
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
Re: [PR] [KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop … [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6779:
URL: https://github.com/apache/kyuubi/pull/6779#issuecomment-2469930738
@Madhukar525722 sorry for late reply, I'm quite busy these days, will take a look soon
--
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
Re: [PR] [KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop … [kyuubi]
Posted by "Madhukar525722 (via GitHub)" <gi...@apache.org>.
Madhukar525722 commented on PR #6779:
URL: https://github.com/apache/kyuubi/pull/6779#issuecomment-2441799260
Hi @pan3793 , I have considered your suggestion for using kyuubi.session.preferredGroups. Please review, once approved, I will rebase it.
--
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
Re: [PR] [KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop … [kyuubi]
Posted by "christiangierschner (via GitHub)" <gi...@apache.org>.
christiangierschner commented on PR #6779:
URL: https://github.com/apache/kyuubi/pull/6779#issuecomment-3772364195
Hi,
We would like this feature very much...would that be possible?
--
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
Re: [PR] [KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop … [kyuubi]
Posted by "Madhukar525722 (via GitHub)" <gi...@apache.org>.
Madhukar525722 commented on PR #6779:
URL: https://github.com/apache/kyuubi/pull/6779#issuecomment-2453124068
Hi @pan3793 , Please review the change, integration tests failed due to timeout. In previous it was green
--
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
Re: [PR] [KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop … [kyuubi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6779:
URL: https://github.com/apache/kyuubi/pull/6779#issuecomment-2439693785
## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/6779?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `0%` with `18 lines` in your changes missing coverage. Please review.
> Project coverage is 0.00%. Comparing base [(`d3520dd`)](https://app.codecov.io/gh/apache/kyuubi/commit/d3520ddbcea96ec55c525600126047c44c7adb35?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`66955cf`)](https://app.codecov.io/gh/apache/kyuubi/commit/66955cff7ab7574525c9c42341240722f5f3e1e2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
| [Files with missing lines](https://app.codecov.io/gh/apache/kyuubi/pull/6779?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...rg/apache/kyuubi/session/HadoopGroupProvider.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6779?src=pr&el=tree&filepath=kyuubi-server%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fsession%2FHadoopGroupProvider.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0hhZG9vcEdyb3VwUHJvdmlkZXIuc2NhbGE=) | 0.00% | [11 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6779?src=pr&el=tree&filepath=kyuubi-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fconfig%2FKyuubiConf.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ain/scala/org/apache/kyuubi/engine/EngineRef.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6779?src=pr&el=tree&filepath=kyuubi-server%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fengine%2FEngineRef.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvRW5naW5lUmVmLnNjYWxh) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #6779 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 687 687
Lines 42439 42455 +16
Branches 5793 5798 +5
======================================
- Misses 42439 42455 +16
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/kyuubi/pull/6779?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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
Re: [PR] [KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop … [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #6779:
URL: https://github.com/apache/kyuubi/pull/6779#issuecomment-2440057411
To support preferred the group name in session conf, I would suggest to do the following:
1. add a session conf key in `KyuubiReservedKeys` rather than in adding a server side config for a global preferred group (you are not even using its value anyway).
2. add a server config for choosing the select policy, eg. 'head', 'prefered_session_conf'
And I have worries about using the session conf will interference choosing the engine reference, as it's easy to be changed at runtime. Is there any better approach with solid connection variables?
--
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
Re: [PR] [KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop … [kyuubi]
Posted by "Madhukar525722 (via GitHub)" <gi...@apache.org>.
Madhukar525722 commented on PR #6779:
URL: https://github.com/apache/kyuubi/pull/6779#issuecomment-2439693087
Hi @pan3793 @bowenliang123 . Please review the implementation. Thanks
--
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
Re: [PR] [KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop … [kyuubi]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6779:
URL: https://github.com/apache/kyuubi/pull/6779#issuecomment-2807848384
Thanks for the PR! This PR is being closed due to inactivity. This isn't a judgement on the merit of the PR in any way. If this is still an issue with the latest version of Kyuubi, please reopen it and ask a committer to remove the Stale tag!
Thank you for using Kyuubi!
--
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
Re: [PR] [KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop … [kyuubi]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #6779: [KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop …
URL: https://github.com/apache/kyuubi/pull/6779
--
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