-
Notifications
You must be signed in to change notification settings - Fork 919
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.
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
[KYUUBI #6402]: engine.share.level=GROUP enable for a list of hadoop … #6779
base: master
Are you sure you want to change the base?
Conversation
b507477
to
66955cf
Compare
Hi @pan3793 @bowenliang123 @turboFei . Please review the implementation. Thanks |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6779 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 687 687
Lines 42439 42459 +20
Branches 5793 5799 +6
======================================
- Misses 42439 42459 +20 ☔ View full report in Codecov by Sentry. |
To support preferred the group name in session conf, I would suggest to do the following:
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? |
@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
seems there is no much differences from the existing properties like |
Hi @pan3793 , I have considered your suggestion for using kyuubi.session.preferredGroups. Please review. |
Hi @pan3793 , Please review the change, integration tests failed due to timeout. In previous it was green |
@Madhukar525722 sorry for late reply, I'm quite busy these days, will take a look soon |
…groups
🔍 Description
Issue References 🔗
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 :
Types of changes 🔖
Test Results🧪
When PREFERRED_GROUPS were 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
User: madlnu
When valid PREFERRED_GROUPS were defined
Spark application name: kyuubi_GROUP_SPARK_SQL_kyuubi_test_b_default_be9a16a8-be38-4ab6-bee9-1934f8556f18
application ID: application_1728291907264_43968
User: madlnu
When no PREFERRED_GROUPS were matching, 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
User: madlnu
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
User: madlnu
Checklist 📝
Be nice. Be informative.