Skip to content

[ZEPPELIN-619] Shared Resource pool across interpreter processes#655

Closed
Leemoonsoo wants to merge 17 commits into
apache:masterfrom
Leemoonsoo:resource_pool
Closed

[ZEPPELIN-619] Shared Resource pool across interpreter processes#655
Leemoonsoo wants to merge 17 commits into
apache:masterfrom
Leemoonsoo:resource_pool

Conversation

@Leemoonsoo
Copy link
Copy Markdown
Member

What is this PR for?

This is sub task of https://issues.apache.org/jira/browse/ZEPPELIN-533.
It provides shared resource pool to exchange data across interpreter processes.

What type of PR is it?

Feature

Is there a relevant Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-619

How should this be tested?

create two different spark interpreter settings.
create two different notebooks each bind different spark interpreter setting.

put an object from one notebook.
read the object from the other notebook. (from the other interpreter process)

See screenshot

Screenshots (if appropriate)

resource_pool

Questions:

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

@Leemoonsoo Leemoonsoo force-pushed the resource_pool branch 3 times, most recently from b146820 to 9f1105d Compare January 19, 2016 05:13
@Leemoonsoo Leemoonsoo closed this Jan 19, 2016
@Leemoonsoo Leemoonsoo reopened this Jan 19, 2016
@doanduyhai
Copy link
Copy Markdown
Contributor

A question @Leemoonsoo, it resource sharing only available through the ZeppelinContext object of the Spark interpreter or is it also available to other interpreters ?

@Leemoonsoo
Copy link
Copy Markdown
Member Author

@doanduyhai
ZeppelinContext expose convenient API for ResourcePool for scala repl.
ResourcePool is available through InterpreterContext.getResourcePool() and InterpreterContext is injected into every Interpreter.interpret() call.
So it is available to all other interpreters.

@doanduyhai
Copy link
Copy Markdown
Contributor

Great news! We have finally a way to share variables between interpreters!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using ConcurrentHashMap to remove the synchronize block.

Same remark for other synchronize blocks within this class

@doanduyhai
Copy link
Copy Markdown
Contributor

Another question, now that we have a resource pull to share data between interpreters, should we deprecate AngularObjectRegistry and use resource pool instead ?

I have the feeling that resource pool is more general purpose. Furthermore with AngularObject we have the notion of scope (paragraph or note) which we don't have with resource pool. It helps segregate data properly

Anyway it is just a question, it's not blocking for this PR

@Leemoonsoo
Copy link
Copy Markdown
Member Author

@doanduyhai Thanks for review and i think i addressed all your comment. Please take a look.

Regarding ResourcePool and AngularObjectRegistry, here's differences

  • ResourcePool is not propagating object from/to front-end, but AngularObjectRegistry do.
  • ResourcePool is not persisted into note.json but AngularObjectRegistry does.
  • AngularObjectRegistry has scope (paragraph/notebook) but ResourcePool does not.

I agree, if we improve ResourcePool more and replace AngularObjectRegistry, that would be simpler and better.

@doanduyhai
Copy link
Copy Markdown
Contributor

@Leemoonsoo Thanks for the detailed explanation of differences between AngularObjectRegistry and ResourcePool. I guess that the AngularObjectRegistry is still very relevant since it has a scope system and is saved with notebooks in json.

Otherwise, the code looks good to me so +1 for merge

@Leemoonsoo
Copy link
Copy Markdown
Member Author

Thanks @doanduyhai for the review.
Merge if there're no more discussions.

@asfgit asfgit closed this in ddf2c89 Feb 1, 2016
asfgit pushed a commit that referenced this pull request Feb 1, 2016
### What is this PR for?
Master branch build failure after merging #655 #591

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

### 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 #680 from Leemoonsoo/ZEPPELIN-619_followup and squashes the following commits:

beac930 [Lee moon soo] Fix test
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.

2 participants