Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/4135#issuecomment-78550324
I chatted with @tdas about this and I think we've arrived at a better
understanding of this problem:
- The `onTaskCompletion` approach that I suggested will not work because
that callback will only be invoked after the task has finished running, but the
task will not be able to finish until we stop the receiver.
- We could modify the receiver to respond to `TaskContext.isInterrupted()`,
but the current implementation here isn't ideal since it involves busy-looping.
It sounds like your original approach of adding a new callback on
interruption is the right fix, since that callback can just call
`Receiver.stop()` and we can continue to use the countdown latch to avoid
polling / busy-waiting. However, I think that the original name for the
callback was misleading / confusing, which led me onto this derail: the name
`addTaskKilledListener` is confusing because it sounds like it would be
subsumed by `onCompleteCallback`. Besides this naming confusion, though, I
think the original approach is actually fine.
Therefore, my suggestion is to keep the unit tests that you've added (which
look good to me) and revert back to the `onKilledCallback` approach, but change
the name to be `addTaskInterruptionListener`. Because the existing methods /
naming was confusing, it might also be nice to add a state transition diagram
somewhere which explains the order in which the callbacks are invoked (or some
much more explicit comments).
Sorry for the derail and thanks for being patient with 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 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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
