Github user uce commented on the pull request:
https://github.com/apache/flink/pull/896#issuecomment-121046784
Nice piece of work! I agree with Stephan's points. I think it was good to
address them. :) I've added some minor comments inline.
Regarding the high-level comments:
- Wasn't the rationale to have a separate message for the heartbeat and the
accumulators to keep the heartbeat messages small? What do you think? In any
case, I agree that it makes sense to be able to configure this.
- I would add a unit test for the record reader/writer accumulator results.
- There seems to be a 1:1 correspondence between the reporter and the
internal metrics. Is the idea to have multiple different reporters in the
future (for different types of metrics etc.) or is a long reporter sufficient?
---
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.
---
