Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1966#issuecomment-220275206
I like the addition. Two things, however, that I am not sure about:
1. The `Histogram` uses a `LongHistogram` internally, which results in a
lot of wrapping and converting. Each accumulator report (each heartbeat) needs
to do the conversion. I think the Histogram should rather hold the proper (int,
int) map directly.
2. I am skeptical about failing hard in the Histogram on an integer
overflow. These kind of hard failures in utility types are always tricky. A
program causing an overflow will result in a non-recoverable failure (it will
always overflow again). For streaming programs, that is not a nice property. I
would actually rather try to deprecate the int histogram (let it overflow) and
encourage to replace it over time completely with the long histogram.
---
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.
---
