Github user squito commented on the pull request:
https://github.com/apache/spark/pull/8760#issuecomment-185965488
@kayousterhout @markhamstra one other thing to follow up on was the earlier
idea of using speculation to test out whether blacklisted executors can be
added back. While I think its a neat _idea_, I really do not think it needs to
be included here. First of all, speculation has always been somewhat poorly
tested and its taken a while to root out some race conditions -- I dont' think
users would want speculation turned on without explicitly requesting it, and I
think we can still do useful blacklisting without requiring users to turn it
on. Second, I think it could lead to lots of user confusion from the errors
generated by these speculative tasks. Third, it would add a lot of complexity,
and it can always be added as an option later on.
I think the other concern is how this will affect scheduler throughput. I
ran the spark-perf scheduler throughput tests (just locally), and didn't see
any significant performance difference. However those tests aren't really
stressing this part of the code, in particular there aren't failed executors,
so its probably worth thinking of another test for this. The per-task overhead
introduced with the lock-less approach will just be one call to
`clock.getTimeMillis()`, which is pretty negligible, plus an occasional "blip"
in the scheduling when the blacklists are updated. That would be non-zero but
I also doubt it would be a problem, however some profiling is in order.
---
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]
