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 >
