Implemented in this PR : https://github.com/apache/incubator-zeppelin/pull/678
I did not change the addAndNotifyRemoteProcess() and its remove corresponding methods. For now I just let them as they are. We may think about improving the design when we refactor the code later On Sun, Jan 24, 2016 at 9:00 PM, moon soo Lee <m...@apache.org> wrote: > Adding angularRegistryPush() and pushing angularobjects on interpreter > initialize sounds like a good plan. > > But overriding add() and removing addAndNotifyRemoteProcess may need > something more. Because update from interpreter side sent to zeppelin > server will be update RemoteAngularObjectRegistry using this add() method. > If add() overrides, then it updates the interpreter process back, which is > unnecessary. > > Thanks, > moon > > On 2016년 1월 24일 (일) at 오전 9:11 DuyHai Doan <doanduy...@gmail.com> wrote: > > > There is a elegant solution for this. > > > > I'm adding a new Thrift method called angularRegistryPush(). > > > > The idea is that every time the RemoteInterpreter start a new remote > > Interpreter, after the call to client.createInterpreter(...): > > > > 1. we retrieve the ZeppelinServer-local angular registry for this > > interpreter group > > 2. we serialized the whole registry Map<String, Map<String, > AngularObject>> > > as JSON using GSON library > > 3. we push this JSON with Thrift calling client.angularRegistryPush() > > > > At initialization time, there should be normally 0 registered listener or > > watcher on the AngularObject so we do not need to push them to remote > > interpreter > > > > With this process in place, we can safely rename > > addAndNotifyRemoteProcess() > > of RemoteAngularObjectRegistry to add() to override the default > > add() behaviour of the base class AngularObjectRegistry. Same renaming > for > > removeXXX() methods. > > > > Inside each method of RemoteAngularObjectRegistry, we check : > > > > a. if the remote interpreter process is started, add/remove the value > > to/from local HashMap and push to remote using Thrift client > > b. else, only add/remove the value to/from local HashMap, the push of the > > registry to remote interpreter will be done once it is started by > > RemoteInterpreter > > class, as described above > > > > WDYT ? > > > > > > > > > > On Sun, Jan 24, 2016 at 1:16 AM, moon soo Lee <m...@apache.org> wrote: > > > > > Hi DuyHai, > > > > > > Thanks for interest to AngularObject. > > > > > > In short, RemoteAngularObjectRegistry is proxy for > AngularObjectRegistry > > > that running in interpreter process. > > > > > > Once an AngularObject is created, the object stays in both > > > RemoteAngularObjectRegistry (in zeppelinServer) and > AngularObjectRegistry > > > (in interpreter process). > > > > > > Update of AngularObject from front-end side propagated to > > > AngularObjectRegistry(in interpreter process) through > > > RemoteAngularObjectRegistry (in zeppelinServer). > > > > > > Update of AngularObject from interpreter side propagated to the > front-end > > > through RemoteAngularObjectRegistry (in zeppelinServer) > > > > > > Therefore, RemoteAngularObjectRegistry supposed to be always up to > dated > > > and calling get() will return up-to-dated object from local hash map. > > > > > > One of the reason keeping a copy of HashMap in > > RemoteAngularObjectRegistry > > > (in ZeppelinServer) is, to save those angular objects into note.json > when > > > notebook is persisted. > > > > > > Right, ideally, addAndNotifyRemoteProcess() should be renamed to add() > > and > > > override the add() method. the same for removeAndNotifyRemoteProcess(). > > but > > > i couldn't manage to write code in that way when i was implementing > those > > > classes. > > > > > > > > > About loadNoteFromRepo(), saved angularObjects in note.json should be > > > restored when notebook is loaded. But interpreters are lazy > initialized. > > So > > > the angularObject in note.json can not be created in interpreter > process > > > just after Zeppelin server is created. Current implementation restore > the > > > object at least in ZeppelinServer side, so front-end can read those > > values. > > > But if those angularObjects read from note.json can be restored to lazy > > > initialized interpreter, that would be better. > > > > > > Thanks, > > > moon > > > > > > On Sat, Jan 23, 2016 at 12:36 PM DuyHai Doan <doanduy...@gmail.com> > > wrote: > > > > > > > It's even worst than what I though, addAndNotifyRemoteProcess() is > > never > > > > used in production code, only called by an unit test. > > > > > > > > The method Notebook.loadNoteFromRepo() just mention it in a comment: > > > > > > > > // at this point, remote interpreter process is not > created. > > > > // so does not make sense add it to the remote. > > > > // > > > > // therefore instead of addAndNotifyRemoteProcess(), need > to > > > use > > > > add() > > > > // that results add angularObject only in ZeppelinServer > side > > > not > > > > remoteProcessSide > > > > > > > > On Sat, Jan 23, 2016 at 9:14 PM, DuyHai Doan <doanduy...@gmail.com> > > > wrote: > > > > > > > > > Hello guys > > > > > > > > > > While developing a new feature, I can see some inconsistencies in > > the > > > > > design of the AngularObjecRegistry and RemoteAngularObjectRegistry > > > > classes. > > > > > > > > > > The remote class extends the base AngularObjecRegistry class, so > it > > > also > > > > > inherits the values HashMap. > > > > > > > > > > The problem is that this value HashMap is JVM-local, therefore, > > there > > > > are > > > > > some methods like addAndNotifyRemoteProcess() > > > > > and removeAndNotifyRemoteProcess() to add/remove variables to/from > > the > > > > > remote registry. > > > > > > > > > > So far so good. > > > > > > > > > > But then there is no method to read/check the presence of a > variable > > > > from > > > > > the remote registry. Calling get() only read the local HashMap .... > > > > > > > > > > Ideally, the addAndNotifyRemoteProcess() should be renamed add() > and > > > > > override the add() method of the base AngularObjecRegistry class. > > Same > > > > idea > > > > > for removeAndNotifyRemoteProcess() method. If we are dealing with a > > > > remote > > > > > registry, that make senses that we do not use the JVM-local > HashMap. > > > > > > > > > > What do you think about the idea ? Is there any use-case where we > DO > > > > need > > > > > to access the local HashMap for a RemoteAngularObjectRegistry > > instance > > > ? > > > > > > > > > > Remarks are welcomed > > > > > > > > > > Regards > > > > > > > > > > > > > > >
