Thanks, I got it ) Then I'm going to follow the way you suggested - to ignore an OUTPUT_UPDATE_ALL event after a job is finished, because we can't guarantee that all events will be delivered before 'interpret(..)' method ends - we don't know how many events we should receive from the concrete interpreter.
Should we ignore latecomer OUTPUT_APPEND and OUTPUT_UPDATE events as well? Regards, Alexander -----Original Message----- From: moon soo Lee [mailto:m...@apache.org] Sent: Tuesday, February 07, 2017 8:28 AM To: dev@zeppelin.apache.org Subject: Re: ZEPPELIN-1856 onOutputClear() method is connected with InterpreterOutput.clear() method. clear() method is designed for the use case inside of Interpreter.interpret() like, InterpreterOutput.write("something"); // notebook displays "something" InterpreterOutput.clear(); // notebook clears "something" InterpreterOutput.write("something else"); // notebook displays "something else" For example, it can be useful when some interpreter want to print progress information (like logs, etc) while it's running, and when it finishes, clear output and display result. Will there be a way to fix flaky test ZEPPELIN-1856 while keeping this ability? Thanks, moon On Mon, Feb 6, 2017 at 6:08 PM Alexander Shoshin <alexander_shos...@epam.com> wrote: > Hi moon, > Thanks for the answer. > > A agree that one of possible ways to solve the problem is to > synchronize OUTPUT_UPDATE_ALL event delivery and 'interpret()' result return. > > But I still don't understand why we need to clear 'Paragraph.results' > from 'onOutputClear()' method. I thought that the main goal of this > method is to clear results on UI through 'broadcastParagraph(..)'. And > if we also try to clear 'Paragraph.results' from this method we can > find that it is either already cleared (by 'InterpreterContext' > initialization in > 'Paragraph.jobRun()') or filled with 'interpret()' result. After > ignoring OUTPUT_UPDATE_ALL event in case of FINISHED status we will > come in situation where we always clear an empty 'Paragraph.results'. > Does it make sense? > > Regards, > Alexander > > -----Original Message----- > From: moon soo Lee [mailto:m...@apache.org] > Sent: Saturday, February 04, 2017 4:30 AM > To: dev@zeppelin.apache.org > Subject: Re: ZEPPELIN-1856 > > Hi, > > Thanks for working on issue ZEPPELIN-1856. > I think removing 'note.clearParagraphOutput(paragraphId);' from > 'onOutputClear()' is not the best solution, because without the line > 'onOutputClear()' does not do anything useful. > > InterpreterContext is constructed before call > RemoteInterpreterServer.interpret(). So OUTPUT_UPDATE_ALL supposed to > arrive before RemoteInterpreterServer.interpret() returns. Apparently, > it looks like it doesn't. So i think we need to tackle this problem. > > OUTPUT_UPDATE_ALL event is delivered via > RemoteInterpreterEventClient/RemoteInterpreterEventPoller, while > RemoteInterpreterServer.interpret() does not, and > RemoteInterpreterEventClient/RemoteInterpreterEventPoller does not > guarantee message deliver before interpret() returns. That's origin of > problem i think. > > Therefore, proper way solving this problem is add some mechanism that > guarantee RemoteInterpreterEventClient/RemoteInterpreterEventPoller > deliver the OUTPUT_UPDATE_ALL event before interpret() returns. Or > simple workaround could be ignore OUTPUT_UPDATE_ALL event when > paragraph becomes FINISHED status (after interpret() returns). > > Thanks, > moon > > On Sat, Feb 4, 2017 at 12:02 AM Alexander Shoshin < > alexander_shos...@epam.com> wrote: > > > Hi, > > > > I'm working on issue > > https://issues.apache.org/jira/browse/ZEPPELIN-1856 > > and I found that we receive a NullPointerException sometimes because > > a paragraph result is cleared twice when we run a job. First > > Paragraph.result is cleared just before running > > RemoteInterpreter.interpret(..) and this is ok. But then we receive > > an OUTPUT_UPDATE_ALL event from the RemoteInterpreterServer and set > > Paragraph.result to null again that may lead to a > > NullPointerException if Paragraph.result was already filled by > > RemoteInterpreter.interpret(..) responce. > > > > To resolve this problem we need to remove > > note.clearParagraphOutput(paragraphId) line from the onOutputClear() > > method in NotebookServer.java class: > > > > @Override > > public void onOutputClear(String noteId, String paragraphId) { > > Notebook notebook = notebook(); > > final Note note = notebook.getNote(noteId); > > note.clearParagraphOutput(paragraphId); // this line seems to be > > wrong > > Paragraph paragraph = note.getParagraph(paragraphId); > > broadcastParagraph(note, paragraph); } > > > > This method is called only when > > RemoteInterpreterServer.interpret(..) > > initializes an InterpreterContext and sends an OUTPUT_UPDATE_ALL event. > > Is it safe to remove this line or I miss something? It was added by > > https://github.com/apache/zeppelin/pull/1658. > > > > Thanks, > > Alexander > > >
