-
Notifications
You must be signed in to change notification settings - Fork 2.8k
ZEPPELIN-1577. LivyInterpreter should not use FIFOScheduler #1557
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
Conversation
|
Will add test after #1462 |
d9f810c to
f00682c
Compare
|
@prabhjyotsingh @felixcheung Please help review. |
|
Probably something @Leemoonsoo should review |
Leemoonsoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great.
If new scheduler implementation FIFOPerUserScheduler has very basic unittest that make sure creates FIFOScheduler per user, that would be even better.
| private SchedulerListener listener; | ||
| boolean terminate = false; | ||
| private String name; | ||
| private int maxUser = 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like unused var. Is it?
|
Perhaps for this issue and other similar issues related to multiple users, the mode for the livy interpreter should be scoped instead of shared. If we run 1 interpreter per user then that matches the end to end flow wrt sessions. Then interpreters will have their own queue and not need this workaround. Also one user restarting their interpreter would not affect existing interpreter instances for others users. Is that a correct understanding? |
|
@bikassaha Good point. I Agree, that approach seems better. @zjffdu What do you think? |
|
Yes, that would be better. I think currently zeppelin hard coded the default mode (shared mode), there's no way to set the default mode for interpreter, we may add one option in interpreter-setting.json. And also we may add one option to restrict the mode usage as for livy only scoped is valid. |
|
@zjffdu Isolated would also be valid but perhaps an overkill. So default of scoped makes sense. |
|
Exactly. Adding option in interpreter-setting.json and restrict mode usage is great idea. Similar approaches has been made in ZEPPELIN-1026 for editor mode restriction. |
|
Close this one, and open #1585 to allow user to set default mode in interpreter-setting.json |
What is this PR for?
This PR create a new scheduler FIFOPerUserScheduler. So that each user's request is isolated. and it is FIFOSchduler per user.
What type of PR is it?
[Bug Fix | Improvement]
Todos
What is the Jira issue?
How should this be tested?
Tested manually
Screenshots (if appropriate)
Questions: