Github user uce commented on the pull request:
https://github.com/apache/flink/pull/570#issuecomment-89823498
Nice to see that you picked this up again. :-) I know from experience that
it can be tricky to port the old RPC calls to Akka msgs, so kudos. ;-)
- I actually agree with your reformatings, but we had a discussion recently
(http://apache-flink-incubator-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Issues-with-heterogeneity-of-the-code-td4292.html#a4443)
to refrain from reformattings. I think it makes it harder to review your
changes. We haven't really written this down somewhere (this is actually a good
reminder that we need to do this), but you should keep it in mind.
- I only had a quick look at the hack you pointed out. I think we should
*not* merge it in the current state (in any case I would vote to postpone
merging this until *after* the upcoming milestone release). Can you provide
more information about what kind of Exception is thrown? Waiting for 10 ms is
not robust against different timings on different machines.
I think the blocking Await is OK in this case. We should add tests for the
basic JM <-> IterationHeadTask interaction though.
---
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.
---
