You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by mo...@apache.org on 2016/11/21 16:30:31 UTC

zeppelin git commit: ZEPPELIN-1653: Make UI responsive

Repository: zeppelin
Updated Branches:
  refs/heads/master f6995738a -> 1375379a2


ZEPPELIN-1653: Make UI responsive

### What is this PR for?
When we do the following operations, the entire notebook is transmitted back to client.
1) Insert paragraph
2) Remove paragraph
3) Update paragraph
4) Paragraph status update
5) commit paragraph
When the json becomes larger(which happens when large  output is stored in the json), the time to transfer the notebook to client is directly proportional.(we can check this in chrome browser-> dev console-> select ws request-> on the right pane that opens, select frames tab and observe the length column)

And since the UI update is based on this new transmitted json, the UI seems unresponsive/laggy.
In this PR, Making the updates selective.(Not sending the complete json back to client)

### What type of PR is it?
Improvement

### Todos
NA

### What is the Jira issue?
ZEPPELIN-1653

### How should this be tested?

-In chrome browser-> dev console-> select ws request-> on the right pane that opens, select frames tab and observe  *NOTE* message.
We should not see the message for the above mentioned operations except when the notebook is loaded is for first time
-    We should not see *GET_NOTE* log message for above operations.

### Screenshots
NA

### Questions:
* Does the licenses files need update?No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: karuppayya <ka...@gmail.com>
Author: Karup <ka...@outlook.com>

Closes #1624 from karup1990/ZEPPELIN-1653 and squashes the following commits:

bbdfc9a [karuppayya] Address review comments
4cd9dcd [karuppayya] fix lookandfeel config
c99383a [Karup] Refactor code , dont broadcast note


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/1375379a
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/1375379a
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/1375379a

Branch: refs/heads/master
Commit: 1375379a28f38aca8001ed2bce6258730716b652
Parents: f699573
Author: karuppayya <ka...@gmail.com>
Authored: Thu Nov 17 22:46:15 2016 +0530
Committer: Lee moon soo <mo...@apache.org>
Committed: Mon Nov 21 08:30:15 2016 -0800

----------------------------------------------------------------------
 .../apache/zeppelin/socket/NotebookServer.java  |  48 ++++++---
 .../src/app/notebook/notebook.controller.js     | 108 ++++++-------------
 .../websocketEvents/websocketEvents.factory.js  |   8 ++
 .../zeppelin/notebook/socket/Message.java       |   4 +
 4 files changed, 83 insertions(+), 85 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1375379a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
index c434ffe..ebc5755 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
@@ -568,6 +568,20 @@ public class NotebookServer extends WebSocketServlet implements
     broadcast(noteId, new Message(OP.INTERPRETER_BINDINGS).put("interpreterBindings", settingList));
   }
 
+  public void broadcastParagraph(Note note, Paragraph p) {
+    broadcast(note.getId(), new Message(OP.PARAGRAPH).put("paragraph", p));
+  }
+
+  private void broadcastNewParagraph(Note note, Paragraph para) {
+    LOG.info("Broadcasting paragraph on run call instead of note.");
+    int paraIndex = note.getParagraphs().indexOf(para);
+    broadcast(
+        note.getId(),
+        new Message(OP.PARAGRAPH_ADDED)
+          .put("paragraph", para)
+          .put("index", paraIndex));
+  }
+
   public void broadcastNoteList(AuthenticationInfo subject, HashSet userAndRoles) {
     if (subject == null) {
       subject = new AuthenticationInfo(StringUtils.EMPTY);
@@ -715,7 +729,10 @@ public class NotebookServer extends WebSocketServlet implements
 
       AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
       note.persist(subject);
-      broadcastNote(note);
+      broadcast(note.getId(), new Message(OP.NOTE_UPDATED)
+          .put("name", name)
+          .put("config", config)
+          .put("info", note.getInfo()));
       broadcastNoteList(subject, userAndRoles);
     }
   }
@@ -857,7 +874,7 @@ public class NotebookServer extends WebSocketServlet implements
     p.setTitle((String) fromMessage.get("title"));
     p.setText((String) fromMessage.get("paragraph"));
     note.persist(subject);
-    broadcast(note.getId(), new Message(OP.PARAGRAPH).put("paragraph", p));
+    broadcastParagraph(note, p);;
   }
 
   private void cloneNote(NotebookSocket conn, HashSet<String> userAndRoles,
@@ -930,9 +947,12 @@ public class NotebookServer extends WebSocketServlet implements
 
     /** We dont want to remove the last paragraph */
     if (!note.isLastParagraph(paragraphId)) {
-      note.removeParagraph(subject.getUser(), paragraphId);
+      Paragraph para = note.removeParagraph(subject.getUser(), paragraphId);
       note.persist(subject);
-      broadcastNote(note);
+      if (para != null) {
+        broadcast(note.getId(), new Message(OP.PARAGRAPH_REMOVED).
+                                  put("id", para.getId()));
+      }
     }
   }
 
@@ -950,9 +970,9 @@ public class NotebookServer extends WebSocketServlet implements
           userAndRoles, notebookAuthorization.getWriters(noteId));
       return;
     }
-
     note.clearParagraphOutput(paragraphId);
-    broadcastNote(note);
+    Paragraph paragraph = note.getParagraph(paragraphId);
+    broadcastParagraph(note, paragraph);
   }
 
   private void completion(NotebookSocket conn, HashSet<String> userAndRoles, Notebook notebook,
@@ -1237,7 +1257,9 @@ public class NotebookServer extends WebSocketServlet implements
 
     note.moveParagraph(paragraphId, newIndex);
     note.persist(subject);
-    broadcastNote(note);
+    broadcast(note.getId(), new Message(OP.PARAGRAPH_MOVED)
+              .put("id", paragraphId)
+              .put("index", newIndex));
   }
 
   private void insertParagraph(NotebookSocket conn, HashSet<String> userAndRoles,
@@ -1254,9 +1276,9 @@ public class NotebookServer extends WebSocketServlet implements
       return;
     }
 
-    note.insertParagraph(index);
+    Paragraph newPara = note.insertParagraph(index);
     note.persist(subject);
-    broadcastNote(note);
+    broadcastNewParagraph(note, newPara);
   }
 
   private void cancelParagraph(NotebookSocket conn, HashSet<String> userAndRoles, Notebook notebook,
@@ -1313,7 +1335,8 @@ public class NotebookServer extends WebSocketServlet implements
     boolean isTheLastParagraph = note.isLastParagraph(p.getId());
     if (!(text.trim().equals(p.getMagic()) || Strings.isNullOrEmpty(text)) &&
         isTheLastParagraph) {
-      note.addParagraph();
+      Paragraph newPara = note.addParagraph();
+      broadcastNewParagraph(note, newPara);
     }
 
     AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
@@ -1631,8 +1654,9 @@ public class NotebookServer extends WebSocketServlet implements
           LOG.error(e.toString(), e);
         }
       }
-      notebookServer.broadcastNote(note);
-
+      if (job instanceof Paragraph) {
+        notebookServer.broadcastParagraph(note, (Paragraph) job);
+      }
       try {
         notebookServer.broadcastUpdateNoteJobInfo(System.currentTimeMillis() - 5000);
       } catch (IOException e) {

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1375379a/zeppelin-web/src/app/notebook/notebook.controller.js
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/app/notebook/notebook.controller.js b/zeppelin-web/src/app/notebook/notebook.controller.js
index f308047..263b682 100644
--- a/zeppelin-web/src/app/notebook/notebook.controller.js
+++ b/zeppelin-web/src/app/notebook/notebook.controller.js
@@ -360,86 +360,50 @@
       return noteCopy;
     };
 
-    var updateNote = function(note) {
-      /** update Note name */
-      if (note.name !== $scope.note.name) {
-        console.log('change note name: %o to %o', $scope.note.name, note.name);
-        $scope.note.name = note.name;
-      }
-
-      $scope.note.config = note.config;
-      $scope.note.info = note.info;
-
-      var newParagraphIds = note.paragraphs.map(function(x) {return x.id;});
-      var oldParagraphIds = $scope.note.paragraphs.map(function(x) {return x.id;});
-
-      var numNewParagraphs = newParagraphIds.length;
-      var numOldParagraphs = oldParagraphIds.length;
-
-      var paragraphToBeFocused;
-      var focusedParagraph;
-      for (var i = 0; i < $scope.note.paragraphs.length; i++) {
-        var paragraphId = $scope.note.paragraphs[i].id;
-        if (angular.element('#' + paragraphId + '_paragraphColumn_main').scope().paragraphFocused) {
-          focusedParagraph = paragraphId;
-          break;
+    var addPara = function(paragraph, index) {
+      $scope.note.paragraphs.splice(index, 0, paragraph);
+      _.each($scope.note.paragraphs, function(para) {
+        if (para.id === paragraph.id) {
+          para.focus = true;
         }
-      }
+      });
+    };
 
-      /** add a new paragraph */
-      if (numNewParagraphs > numOldParagraphs) {
-        for (var index in newParagraphIds) {
-          if (oldParagraphIds[index] !== newParagraphIds[index]) {
-            $scope.note.paragraphs.splice(index, 0, note.paragraphs[index]);
-            paragraphToBeFocused = note.paragraphs[index].id;
-            break;
-          }
-          $scope.$broadcast('updateParagraph', {
-            note: $scope.note, // pass the note object to paragraph scope
-            paragraph: note.paragraphs[index]});
+    var removePara = function(paragraphId) {
+      var removeIdx;
+      _.each($scope.note.paragraphs, function(para, idx) {
+        if (para.id === paragraphId) {
+          removeIdx = idx;
         }
-      }
+      });
+      return $scope.note.paragraphs.splice(removeIdx, 1);
+    };
 
-      /** update or move paragraph */
-      if (numNewParagraphs === numOldParagraphs) {
-        for (var idx in newParagraphIds) {
-          var newEntry = note.paragraphs[idx];
-          if (oldParagraphIds[idx] === newParagraphIds[idx]) {
-            $scope.$broadcast('updateParagraph', {
-              note: $scope.note, // pass the note object to paragraph scope
-              paragraph: newEntry});
-          } else {
-            // move paragraph
-            var oldIdx = oldParagraphIds.indexOf(newParagraphIds[idx]);
-            $scope.note.paragraphs.splice(oldIdx, 1);
-            $scope.note.paragraphs.splice(idx, 0, newEntry);
-            // rebuild id list since paragraph has moved.
-            oldParagraphIds = $scope.note.paragraphs.map(function(x) {return x.id;});
-          }
+    $scope.$on('addParagraph', function(event, paragraph, index) {
+      addPara(paragraph, index);
+    });
 
-          if (focusedParagraph === newParagraphIds[idx]) {
-            paragraphToBeFocused = focusedParagraph;
-          }
-        }
-      }
+    $scope.$on('removeParagraph', function(event, paragraphId) {
+      removePara(paragraphId);
+    });
 
-      /** remove paragraph */
-      if (numNewParagraphs < numOldParagraphs) {
-        for (var oldidx in oldParagraphIds) {
-          if (oldParagraphIds[oldidx] !== newParagraphIds[oldidx]) {
-            $scope.note.paragraphs.splice(oldidx, 1);
-            break;
-          }
-        }
+    $scope.$on('moveParagraph', function(event, paragraphId, newIdx) {
+      var removedPara = removePara(paragraphId);
+      if (removedPara && removedPara.length === 1) {
+        addPara(removedPara[0], newIdx);
       }
+    });
 
-      // restore focus of paragraph
-      for (var f = 0; f < $scope.note.paragraphs.length; f++) {
-        if (paragraphToBeFocused === $scope.note.paragraphs[f].id) {
-          $scope.note.paragraphs[f].focus = true;
-        }
+    $scope.$on('updateNote', function(event, name, config, info) {
+      /** update Note name */
+      if (name !== $scope.note.name) {
+        console.log('change note name to : %o', $scope.note.name);
+        $scope.note.name = name;
       }
-    };
+      $scope.note.config = config;
+      $scope.note.info = info;
+      initializeLookAndFeel();
+    });
 
     var getInterpreterBindings = function() {
       websocketMsgSrv.getInterpreterBindings($scope.note.id);
@@ -856,8 +820,6 @@
 
       if ($scope.note === null) {
         $scope.note = note;
-      } else {
-        updateNote(note);
       }
       initializeLookAndFeel();
       //open interpreter binding setting when there're none selected

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1375379a/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js b/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js
index 71ed7d8..d6ab8f2 100644
--- a/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js
+++ b/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js
@@ -140,6 +140,14 @@
         $rootScope.$broadcast('configurationsInfo', data);
       } else if (op === 'INTERPRETER_SETTINGS') {
         $rootScope.$broadcast('interpreterSettings', data);
+      } else if (op === 'PARAGRAPH_ADDED') {
+        $rootScope.$broadcast('addParagraph', data.paragraph, data.index);
+      } else if (op === 'PARAGRAPH_REMOVED') {
+        $rootScope.$broadcast('removeParagraph', data.id);
+      } else if (op === 'PARAGRAPH_MOVED') {
+        $rootScope.$broadcast('moveParagraph', data.id, data.index);
+      } else if (op === 'NOTE_UPDATED') {
+        $rootScope.$broadcast('updateNote', data.name, data.config, data.info);
       }
     });
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1375379a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/socket/Message.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/socket/Message.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/socket/Message.java
index b6a305c..4f432eb 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/socket/Message.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/socket/Message.java
@@ -147,6 +147,10 @@ public class Message {
     INTERPRETER_SETTINGS,         // [s-c] interpreter settings
     ERROR_INFO,                   // [s-c] error information to be sent
     WATCHER,                      // [s-c] Change websocket to watcher mode.
+    PARAGRAPH_ADDED,              // [s-c] paragraph is added
+    PARAGRAPH_REMOVED,            // [s-c] paragraph deleted
+    PARAGRAPH_MOVED,              // [s-c] paragraph moved
+    NOTE_UPDATED                  // [s-c] paragraph updated(name, config)
   }
 
   public static final Message EMPTY = new Message(null);