Yes, it looks like we can ignore latecomer OUTPUT_APPEND and OUTPUT_UPDATE, as well.
Thanks! moon On Tue, Feb 7, 2017 at 8:57 PM Alexander Shoshin <alexander_shos...@epam.com> wrote: > 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 > > > > > >
