Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/2039
merging if no more comment
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/2039
Code change Looks good to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/2039
ok LGTM.
any comment from other reviewers?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/2039
Ok I wasn't aware of the thread.interrupt semantic in JVM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user DrIgor commented on the issue:
https://github.com/apache/zeppelin/pull/2039
`interrupt` just sets `Thread.isInterrupted` flag and does not immediately
stop the thread. Then thread can check this flag and decide if it needs to
stop. So in fact, termination logic remains
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/2039
From my understanding, setting a flag to let the thread gracefully runs to
end, seems safer than calling `interrupt` on it
---
If your project is set up for it, you can reply to this email
Github user DrIgor commented on the issue:
https://github.com/apache/zeppelin/pull/2039
Ping-ping)
Do you have other questions? Do you think it's better to proper synchronize
`terminate` flag instead?
I reviewed code to understand how zeppelin core works and found
Github user DrIgor commented on the issue:
https://github.com/apache/zeppelin/pull/2039
@felixcheung `terminate` flag is shared across several threads and is
accessed without any synchronization.
And we don't need to implement custom interruption flag because java
already has it
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/2039
for a change like this, it would be useful to open a JIRA and to discuss
why we are making this change
---
If your project is set up for it, you can reply to this email and have your
reply
