Skip to content

Conversation

@Leemoonsoo
Copy link
Member

@Leemoonsoo Leemoonsoo commented Jul 13, 2016

What is this PR for?

Fix for ZEPPELIN-1150.

What type of PR is it?

Bug Fix

Todos

  • - Recreate table on data refresh
  • - Better solution for ZEPPELIN-1078 without performance degrade

What is the Jira issue?

ZEPPELIN-1150

How should this be tested?

Reproduce procedure described in the issue

Questions:

  • Does the licenses files need update? no
  • Is there breaking changes for older versions? no
  • Does this needs documentation? no

@astroshim
Copy link
Contributor

I got a following build error.

[INFO] --- frontend-maven-plugin:0.0.25:grunt (grunt build) @ zeppelin-web ---
[INFO] Running 'grunt build --no-color' in /home/nflabs/tmp/zeppelin/zeppelin-web
[INFO] Running "jscs:all" (jscs) task
[INFO] disallowMultipleLineBreaks: ; and console.log("updateParagraph oldData %o, newData %o. type %o -> %o, mode %o -> %o", $scope.paragraph, data, oldType, newType, oldGraphMode, newGraphMode); should have at most 2 line(s) between them at src/app/notebook/paragraph/paragraph.controller.js :
[INFO]    412 |        isEmpty(data.paragraph.result) !== isEmpty($scope.paragraph.result) ||
[INFO]    413 |        data.paragraph.status === 'ERROR' || (data.paragraph.status === 'FINISHED' && statusChanged) ||
[INFO]    414 |        (!newActiveApp && oldActiveApp !== newActiveApp);
[INFO] -----------------------------------------------------------------^
[INFO]    415 |
[INFO]    416 |
[INFO] >> 1 code style errors found!

@Leemoonsoo
Copy link
Member Author

@astroshim Thanks for testing. Build error is addressed in last commit.

@corneadoug
Copy link
Contributor

corneadoug commented Jul 13, 2016

@Leemoonsoo Tested,
Fix for the table after re-run confirmed.

For the second item, I would keep the solution used in https://issues.apache.org/jira/browse/ZEPPELIN-1078
Since your changes do not bring any improvement regarding performance degrade.
(We still do a renderText, when the text result is the same as before)

@astroshim
Copy link
Contributor

Build error has gone and working good!

@Leemoonsoo
Copy link
Member Author

@corneadoug Try revert line 413 and 473. And then run paragraph multiple times (with table result). In my machine after 2~3 times without 413, 473 change, browser almost freeze.

@Leemoonsoo
Copy link
Member Author

@corneadoug I tried more and it looks like slowing down without 413 and 473 change is not consistently reproducible.

Anyway, logically, 413 and 473 change reduce text rendering from
4 times (rendering 4 status change FINISHED -> PENDING -> RUNNING -> FINISHED)
to
1 time (RUNNING -> FINISHED)

So, i'd like to keep it while it doesn't bring any side effect or increase complexity of code.

@corneadoug
Copy link
Contributor

@Leemoonsoo I tested again, originally I was testing with previous || resultRefreshed
473 does help the text rendering indeed. However I have the same improvement without 413 for both TABLE and TEXT types

@Leemoonsoo
Copy link
Member Author

@corneadoug 413 and 473 "together" are alternative fix for ZEPPELIN-1078 for better performance.

Without 413, applying 473 will bring ZEPPELIN-1078 problem again.

@corneadoug
Copy link
Contributor

@Leemoonsoo I was testing with %spark only, while ZEPPELIN-1078's issue shows with %python.
LGTM

@Leemoonsoo
Copy link
Member Author

Thanks @astroshim @corneadoug for review this!

Merge it into master if there're no more discussions.

@prabhjyotsingh
Copy link
Contributor

I missed this (which solve the same problem as #1182), let me check this.

@prabhjyotsingh
Copy link
Contributor

prabhjyotsingh commented Jul 13, 2016

LGTM 👍

@asfgit asfgit closed this in b1e6e2c Jul 13, 2016
asfgit pushed a commit that referenced this pull request Jul 18, 2016
Fix for [ZEPPELIN-1150](https://issues.apache.org/jira/browse/ZEPPELIN-1150).

Bug Fix

* [x] - Recreate table on data refresh
* [x] - Better solution for [ZEPPELIN-1078](https://issues.apache.org/jira/browse/ZEPPELIN-1078) without performance degrade

[ZEPPELIN-1150](https://issues.apache.org/jira/browse/ZEPPELIN-1150)

Reproduce procedure described in the issue

* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: Lee moon soo <moon@apache.org>

Closes #1171 from Leemoonsoo/ZEPPELIN-1150 and squashes the following commits:

7978f47 [Lee moon soo] remove multiple newlines
b3406b7 [Lee moon soo] Recreate table when (data) is refreshed

(cherry picked from commit b1e6e2c)
Signed-off-by: Mina Lee <minalee@apache.org>

Conflicts:
	zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js
PhilippGrulich pushed a commit to SWC-SENSE/zeppelin that referenced this pull request Aug 8, 2016
### What is this PR for?
Fix for [ZEPPELIN-1150](https://issues.apache.org/jira/browse/ZEPPELIN-1150).

### What type of PR is it?
Bug Fix

### Todos
* [x] - Recreate table on data refresh
* [x] - Better solution for [ZEPPELIN-1078](https://issues.apache.org/jira/browse/ZEPPELIN-1078) without performance degrade

### What is the Jira issue?
[ZEPPELIN-1150](https://issues.apache.org/jira/browse/ZEPPELIN-1150)

### How should this be tested?
Reproduce procedure described in the issue

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

Author: Lee moon soo <moon@apache.org>

Closes apache#1171 from Leemoonsoo/ZEPPELIN-1150 and squashes the following commits:

7978f47 [Lee moon soo] remove multiple newlines
b3406b7 [Lee moon soo] Recreate table when (data) is refreshed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants