[GitHub] [zeppelin] Leemoonsoo opened a new pull request #3356: [ZEPPELIN-3994] Notebook serving
Leemoonsoo opened a new pull request #3356: [ZEPPELIN-3994] Notebook serving URL: https://github.com/apache/zeppelin/pull/3356 ### What is this PR for? This PR implements [notebook serving](https://issues.apache.org/jira/browse/ZEPPELIN-3994) explained [here](https://docs.google.com/document/d/1YA6q8W9yO8a88xzLDYs9zv_fKu2_cnB58rmQbakxi1I/edit?usp=sharing). While this PR is already quite large, front-end implementation will have separate PR. This PR is work in progress. ### How to run 1. Prepare kubernetes cluster 2. Prepare persistentVolumeClaim with name 'task-context-volume-claim' in your kubernetes cluster 3. Build Zeppelin docker image (follow instructions [here](https://github.com/apache/zeppelin/blob/master/docs/quickstart/kubernetes.md) 4. Deploy api router by running `kubectl apply -f k8s/serving-api-router/serving-api-router.yaml` 5. Start Zeppelin on Kubernetes by running `kubectl apply -f k8s/zeppelin-server.yaml` 6. Port forward to the api router (e.g. `kubectl port-forward 8080:80`) and browse `localhost:8080` ### Summary of changes To be updated ### What type of PR is it? Feature ### Todos ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-3994 ### How should this be tested? * First time? Setup Travis CI as described on https://zeppelin.apache.org/contribution/contributions.html#continuous-integration * Strongly recommended: add automated unit tests for any new or changed behavior * Outline any manual steps to test the PR here. ### Questions: * Does the licenses files need update? yes * Is there breaking changes for older versions? no * Does this needs documentation? yes 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zeppelin] zjffdu commented on a change in pull request #3353: [ZEPPELIN-4132]. Spark Interpreter has issue of SPARK-22393
zjffdu commented on a change in pull request #3353: [ZEPPELIN-4132]. Spark Interpreter has issue of SPARK-22393 URL: https://github.com/apache/zeppelin/pull/3353#discussion_r278430016 ## File path: spark/scala-2.11/src/main/scala/org/apache/zeppelin/spark/SparkScala211Interpreter.scala ## @@ -43,7 +44,7 @@ class SparkScala211Interpreter(override val conf: SparkConf, lazy override val LOGGER: Logger = LoggerFactory.getLogger(getClass) - private var sparkILoop: ILoop = _ + private var sparkILoop: SparkILoop = _ Review comment: Scala210Interpreter already did that 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zeppelin] zjffdu commented on a change in pull request #3355: [ZEPPELIN-4133]. Idle sessions are no longer being closed even though TimeoutLifecycleManagement is configured properly
zjffdu commented on a change in pull request #3355: [ZEPPELIN-4133]. Idle
sessions are no longer being closed even though TimeoutLifecycleManagement is
configured properly
URL: https://github.com/apache/zeppelin/pull/3355#discussion_r278429581
##
File path:
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/lifecycle/TimeoutLifecycleManager.java
##
@@ -30,29 +32,30 @@
private long checkInterval;
private long timeoutThreshold;
- private Timer checkTimer;
+ private ScheduledExecutorService checkScheduler;
public TimeoutLifecycleManager(ZeppelinConfiguration zConf) {
this.checkInterval = zConf.getLong(ZeppelinConfiguration.ConfVars
.ZEPPELIN_INTERPRETER_LIFECYCLE_MANAGER_TIMEOUT_CHECK_INTERVAL);
this.timeoutThreshold = zConf.getLong(
ZeppelinConfiguration.ConfVars.ZEPPELIN_INTERPRETER_LIFECYCLE_MANAGER_TIMEOUT_THRESHOLD);
-this.checkTimer = new Timer(true);
-this.checkTimer.scheduleAtFixedRate(new TimerTask() {
- @Override
- public void run() {
+this.checkScheduler = Executors.newScheduledThreadPool(1);
+this.checkScheduler.scheduleAtFixedRate(() -> {
+ try {
long now = System.currentTimeMillis();
for (Map.Entry entry :
interpreterGroups.entrySet()) {
ManagedInterpreterGroup interpreterGroup = entry.getKey();
Long lastTimeUsing = entry.getValue();
- if ((now - lastTimeUsing) > timeoutThreshold ) {
+ if ((now - lastTimeUsing) > timeoutThreshold) {
LOGGER.info("InterpreterGroup {} is timeout.",
interpreterGroup.getId());
interpreterGroup.close();
interpreterGroups.remove(entry.getKey());
}
}
+ } catch (Exception e) {
+LOGGER.warn("Fail to run periodical checking task", e);
Review comment:
Make sense to me. I updated 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] felixcheung commented on a change in pull request #3355: [ZEPPELIN-4133]. Idle sessions are no longer being closed even though TimeoutLifecycleManagement is configured properly
felixcheung commented on a change in pull request #3355: [ZEPPELIN-4133]. Idle
sessions are no longer being closed even though TimeoutLifecycleManagement is
configured properly
URL: https://github.com/apache/zeppelin/pull/3355#discussion_r278419433
##
File path:
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/lifecycle/TimeoutLifecycleManager.java
##
@@ -30,29 +32,30 @@
private long checkInterval;
private long timeoutThreshold;
- private Timer checkTimer;
+ private ScheduledExecutorService checkScheduler;
public TimeoutLifecycleManager(ZeppelinConfiguration zConf) {
this.checkInterval = zConf.getLong(ZeppelinConfiguration.ConfVars
.ZEPPELIN_INTERPRETER_LIFECYCLE_MANAGER_TIMEOUT_CHECK_INTERVAL);
this.timeoutThreshold = zConf.getLong(
ZeppelinConfiguration.ConfVars.ZEPPELIN_INTERPRETER_LIFECYCLE_MANAGER_TIMEOUT_THRESHOLD);
-this.checkTimer = new Timer(true);
-this.checkTimer.scheduleAtFixedRate(new TimerTask() {
- @Override
- public void run() {
+this.checkScheduler = Executors.newScheduledThreadPool(1);
+this.checkScheduler.scheduleAtFixedRate(() -> {
+ try {
long now = System.currentTimeMillis();
for (Map.Entry entry :
interpreterGroups.entrySet()) {
ManagedInterpreterGroup interpreterGroup = entry.getKey();
Long lastTimeUsing = entry.getValue();
- if ((now - lastTimeUsing) > timeoutThreshold ) {
+ if ((now - lastTimeUsing) > timeoutThreshold) {
LOGGER.info("InterpreterGroup {} is timeout.",
interpreterGroup.getId());
interpreterGroup.close();
interpreterGroups.remove(entry.getKey());
}
}
+ } catch (Exception e) {
+LOGGER.warn("Fail to run periodical checking task", e);
Review comment:
perhaps put try catch around `interpreterGroup.close();`? it looks to me
like other calls inside should not fail in most cases. if `close()` is
expensive or could fail, then a try catch just around it could let other groups
a chance to get clean up, instead of skipping all of the rest this is doing now.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] felixcheung commented on a change in pull request #3353: [ZEPPELIN-4132]. Spark Interpreter has issue of SPARK-22393
felixcheung commented on a change in pull request #3353: [ZEPPELIN-4132]. Spark Interpreter has issue of SPARK-22393 URL: https://github.com/apache/zeppelin/pull/3353#discussion_r278391520 ## File path: spark/scala-2.11/src/main/scala/org/apache/zeppelin/spark/SparkScala211Interpreter.scala ## @@ -43,7 +44,7 @@ class SparkScala211Interpreter(override val conf: SparkConf, lazy override val LOGGER: Logger = LoggerFactory.getLogger(getClass) - private var sparkILoop: ILoop = _ + private var sparkILoop: SparkILoop = _ Review comment: shouldn't we do this in all cases? not just Scala211Interpreter? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [zeppelin] zjffdu opened a new pull request #3355: [ZEPPELIN-4133]. Idle sessions are no longer being closed even though TimeoutLifecycleManagement is configured properly
zjffdu opened a new pull request #3355: [ZEPPELIN-4133]. Idle sessions are no longer being closed even though TimeoutLifecycleManagement is configured properly URL: https://github.com/apache/zeppelin/pull/3355 ### What is this PR for? The root cause is that we use Timer to scheduler periodical interpreter checking task. But java Timer has one critical issue that once the Timer thread is crashed, subsequent task will be suppressed. This PR use ScheduledExecutorService instead to fix this issue. ### What type of PR is it? [Bug Fix] ### Todos * [ ] - Task ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-4133 ### How should this be tested? * CI pass ### Screenshots (if appropriate) ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
