Github user zentol commented on the issue:
https://github.com/apache/flink/pull/2112
merging
---
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
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2112
I applied the requested changes.
---
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
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2112
Thanks for your review @zentol. I've addressed your comments modulo the
naming question of `getValue` and the percentile convenience methods. Let's try
to figure out what's the best solution
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2112
Yes the idea is introduce the histogram abstraction which allows us to use
any histogram implementation. At the moment we don't have a need for an
optimized implementation which might change in
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/2112
So we don't want to tie ourselves to DropWizard histograms, but the only
way for users to use a histogram right now _without implementing one themselves
(something that is too much work for us
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2112
We decided to add the histogram metric, because users requested this
feature. The important thing is that the histograms are not used by the runtime
per default, because they indeed don't come
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/2112
-1. I haven't looked very deeply at the actual code but let me share some
thoughts:
I was under the impression that the core of this issue was to implement at
least 1 Histogram that does not
