You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by zj...@apache.org on 2020/11/07 23:51:26 UTC

[zeppelin] branch master updated: [ZEPPELIN-5106]. Only save note when it is loaded or newly created

This is an automated email from the ASF dual-hosted git repository.

zjffdu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/master by this push:
     new 653c3a2  [ZEPPELIN-5106]. Only save note when it is loaded or newly created
653c3a2 is described below

commit 653c3a23eb223db7b379664d31c6e047e974deb3
Author: Jeff Zhang <zj...@apache.org>
AuthorDate: Tue Oct 27 22:14:27 2020 +0800

    [ZEPPELIN-5106]. Only save note when it is loaded or newly created
    
    ### What is this PR for?
    
    This PR is to save note only in 2 cases:
    1. It is loaded.
    2. It is newly created.
    
    Otherwise it will throw NPE because paragraphs is null when note is in unloaded state.
    
    ### What type of PR is it?
    [Bug Fix]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-5106
    
    ### 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
    
    Author: Jeff Zhang <zj...@apache.org>
    
    Closes #3954 from zjffdu/ZEPPELIN-5106 and squashes the following commits:
    
    017c1e92d [Jeff Zhang] only unload note when it is not in in RUNNING
    1c99d393e [Jeff Zhang] save
    3fd02052d [Jeff Zhang] save
    9d2ce6d78 [Jeff Zhang] [ZEPPELIN-5106]. Only save note when it is loaded
---
 .../apache/zeppelin/service/NotebookService.java   |  2 +-
 .../zeppelin/service/SessionManagerService.java    |  6 +++-
 .../java/org/apache/zeppelin/notebook/Note.java    | 42 ++++++++++++++++++----
 .../org/apache/zeppelin/notebook/NoteManager.java  | 13 +++++--
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
index 050b846..917be9e 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
@@ -433,7 +433,7 @@ public class NotebookService {
               return false;
             }
           } catch (Exception e) {
-            throw new IOException("Fail to run paragraph json: " + raw);
+            throw new IOException("Fail to run paragraph json: " + raw, e);
           }
         }
       } finally {
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/SessionManagerService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/SessionManagerService.java
index 479cf19..644b00b 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/SessionManagerService.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/SessionManagerService.java
@@ -171,7 +171,11 @@ public class SessionManagerService {
         if (remoteInterpreterProcess.isRunning()) {
           sessionInfo.setState(SessionState.RUNNING.name());
         } else {
-          sessionInfo.setState(SessionState.STOPPED.name());
+          // if it is running before, but interpreterGroup is not running now, that means the session is stopped.
+          // e.g. InterpreterProcess is terminated for whatever unexpected reason.
+          if (sessionInfo.getState().equals(SessionState.RUNNING.name())) {
+            sessionInfo.setState(SessionState.STOPPED.name());
+          }
         }
       }
     } else {
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java
index 1a1c08a..2a87da6 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java
@@ -124,6 +124,7 @@ public class Note implements JsonSerializable {
 
   /********************************** transient fields ******************************************/
   private transient boolean loaded = false;
+  private transient boolean saved = false;
   private transient InterpreterFactory interpreterFactory;
   private transient InterpreterSettingManager interpreterSettingManager;
   private transient ParagraphJobListener paragraphJobListener;
@@ -190,13 +191,28 @@ public class Note implements JsonSerializable {
    * Release note memory
    */
   public void unLoad() {
-    this.setLoaded(false);
-    this.paragraphs = null;
-    this.config = null;
-    this.info = null;
-    this.noteForms = null;
-    this.noteParams = null;
-    this.angularObjects = null;
+    if (isRunning() || isParagraphRunning()) {
+      LOGGER.warn("Unable to unload note because it is in RUNNING");
+    } else {
+      this.setLoaded(false);
+      this.paragraphs = null;
+      this.config = null;
+      this.info = null;
+      this.noteForms = null;
+      this.noteParams = null;
+      this.angularObjects = null;
+    }
+  }
+
+  public boolean isParagraphRunning() {
+    if (paragraphs != null) {
+      for (Paragraph p : paragraphs) {
+        if (p.isRunning()) {
+          return true;
+        }
+      }
+    }
+    return false;
   }
 
   public boolean isPersonalizedMode() {
@@ -1083,6 +1099,10 @@ public class Note implements JsonSerializable {
     info.remove("startTime");
   }
 
+  /**
+   * Is note running
+   * @return
+   */
   public boolean isRunning() {
     return (boolean) getInfo().getOrDefault("isRunning", false);
   }
@@ -1200,4 +1220,12 @@ public class Note implements JsonSerializable {
   public void setNoteEventListeners(List<NoteEventListener> noteEventListeners) {
     this.noteEventListeners = noteEventListeners;
   }
+
+  public void setSaved(boolean saved) {
+    this.saved = saved;
+  }
+
+  public boolean isSaved() {
+    return saved;
+  }
 }
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
index d487de7..15ec674 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
@@ -168,15 +168,22 @@ public class NoteManager {
 
   /**
    * Save note to NoteManager, it won't check duplicates, this is used when updating note.
+   * Only save note in 2 cases:
+   *  1. Note is new created, isSaved is false
+   *  2. Note is in loaded state. Unload state means its content is empty.
    *
    * @param note
    * @param subject
    * @throws IOException
    */
   public void saveNote(Note note, AuthenticationInfo subject) throws IOException {
-    addOrUpdateNoteNode(note);
-    this.notebookRepo.save(note, subject);
-    note.setLoaded(true);
+    if (note.isLoaded() || !note.isSaved()) {
+      addOrUpdateNoteNode(note);
+      this.notebookRepo.save(note, subject);
+      note.setSaved(true);
+    } else {
+      LOGGER.warn("Try to save note: {} when it is unloaded", note.getId());
+    }
   }
 
   public void addNote(Note note, AuthenticationInfo subject) throws IOException {