Github user CodingCat commented on the pull request:
https://github.com/apache/spark/pull/639#issuecomment-42181908
@markhamstra , thank you very much for the comments and I'm sorry that I
didn't explain clearly in the description of the PR, let me try to explain why
I come up with this solution,
first, I went through the instances you mentioned in SPARK-1620 before I
submit this patch, also I read the API document of Akka...I didn't find a
general way (maybe I missed something) to capture exception in the caller
thread...so I think we need to have a case-specific solution for each instance
there, or we should say that we need to be careful when use
Akka.scheduler.schedule() to do some timer task...but I'm not sure if I'm
correct on this point, so I didn't address exception handle issue in this PR
second, I'm aware of the change of the behaviour in my solution (the
scheduling is called asynchronously). Actually in my opinion, it is even safe
to remove the call (I guess, this line is here is just for more frequent
scheduling?), because schedule() is also called in other places
(relaunchDriver()). (But now, I realized that that call of schedule() in
relaunchDriver() is also a potential problem, as it's called in
completeRecovery, maybe we should change that call to TriggerSchedule also) Or,
we should refactor schedule() method to be just send TriggerSchedule to the
actor, and update the TriggerSchedule handle to do real schedule work, in this
way we can prevent the future risk of calling schedule() in another thread
@aarondav , I think sending a completeRecovery message is simpler than what
I proposed above... I guess we can capture the return value of
akka.scheduler.schedule() and cancel that in postStop as you proposed....
---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
