Hi Till, How would I plugin a custom counter so that I could use the existing MetricGroup and AbstractReporter functionality?
Steve On Friday, June 17, 2016, Till Rohrmann <trohrm...@apache.org <javascript:_e(%7B%7D,'cvml','trohrm...@apache.org');>> wrote: > Hi Steve, > > I thought again about the thread-safe counters and I'm not so sure anymore > whether we should add them or not. The reason is that we could also mark > the Counter class as not final. Then it would be possible to define your > user-defined counters which could be thread-safe. This would reduce the > complexity on the Flink side since covering all metrics use cases might be > difficult. Would that work for you? What do the others think about it? > > Cheers, > Till > > On Thu, Jun 16, 2016 at 3:33 PM, Till Rohrmann <trohrm...@apache.org> > wrote: > >> I agree that dropwizard already offers a lot of functionality and we >> shouldn't reimplement it. Therefore, I've introduced a Flink histogram >> interface which is implemented by a dropwizard histogram wrapper. That way >> Flink has it's own histogram abstraction and dropwizard histograms can be >> used with Flink via this wrapper class. See the PR [1] for more information. >> >> [1] https://github.com/apache/flink/pull/2112 >> >> Cheers, >> Till >> >> On Tue, Jun 14, 2016 at 6:05 PM, Cody Innowhere <e.neve...@gmail.com> >> wrote: >> >>> Counter of dropwizard is thread-safe. >>> I think dropwizard metrics are implemented fairly well and used quite >>> widely in open source projects, I personally on the side of using >>> dropwizard metrics rather than re-implement them, unless for performance >>> reasons. Still, I'm +1 for adding a wrapper on top of dropwizard metrics. >>> >>> On Tue, Jun 14, 2016 at 10:45 PM, Till Rohrmann <trohrm...@apache.org> >>> wrote: >>> >>> > +1 for the thread safe metrics. This should be a rather low hanging >>> fruit >>> > and easily added. >>> > >>> > If we decide to add a histogram, then I would also be in favour of >>> > implementing our own version of a histogram. This avoids adding a hard >>> > dependency on Dropwizard or another metrics library to Flink core. >>> Adding >>> > our own implementation would of course entail to also add a Dropwizard >>> > wrapper for reporting. Thus, if a user component required a Dropwizard >>> > histogram, then one could simply use the wrapper. >>> > >>> > Alternatively, one could also rely on external system to compute >>> > histograms. For example, statsD supports the generation of histograms >>> from >>> > a stream of measurements. However, these histograms couldn't be used >>> within >>> > Flink. >>> > >>> > Implementation wise, the histogram would most likely follow the >>> > implementation of Dropwizard's histogram: >>> > >>> > The basic idea is that a histogram can add samples to a reservoir >>> which it >>> > uses to calculate a set of statistics when queried. The statistics >>> > comprises percentiles, mean, standard deviation and number of >>> elements, for >>> > example. >>> > >>> > The reservoir defines the strategy of how to sample the input stream. >>> There >>> > are different strategies conceivable: Uniform sampling, which >>> constructs a >>> > long term distribution of the seen elements, exponentially decaying >>> > sampling, which favours more recent elements, sliding window or >>> buckets. >>> > >>> > The question is now whether such an implementation already covers most >>> use >>> > cases or whether histograms should support more functionaly. Feedback >>> is >>> > highly appreciated :-) >>> > >>> > Cheers, >>> > Till >>> > >>> > On Mon, Jun 13, 2016 at 6:37 PM, Stephan Ewen <se...@apache.org> >>> wrote: >>> > >>> > > I think it is totally fine to add a "ThreadsafeCounter" that uses an >>> > atomic >>> > > long internally >>> > > >>> > > On Sat, Jun 11, 2016 at 7:25 PM, Steve Cosenza <scose...@twitter.com >>> > >>> > > wrote: >>> > > >>> > > > Also, we may be able to avoid the need for concurrent metrics by >>> > > > configuring each Finagle source to use a single thread. We'll >>> > investigate >>> > > > the performance implications this week and update you with the >>> results. >>> > > > >>> > > > Thanks, >>> > > > Steve >>> > > > >>> > > > >>> > > > On Friday, June 10, 2016, Steve Cosenza <scose...@twitter.com> >>> wrote: >>> > > > >>> > > >> +Chris Hogue who is also working on operationalizing Flink with me >>> > > >> >>> > > >> Hi Stephan, >>> > > >> >>> > > >> Thanks for the background on your current implementations! >>> > > >> >>> > > >> While we don't require a specific implementation for histogram, >>> > counter, >>> > > >> or gauge, it just became clear that we'll need threadsafe >>> versions of >>> > > all >>> > > >> three of these metrics. This is because our messaging source is >>> > > implemented >>> > > >> using Finagle, and Finagle expects to be able to emit stats >>> > concurrently >>> > > >> from its managed threads. >>> > > >> >>> > > >> That being said, if adding threadsafe versions of the Flink >>> counters >>> > is >>> > > >> not an option, we'd also be fine with directly reading and writing >>> > from >>> > > the >>> > > >> singleton Codahale MetricsRegistry that you start up in each >>> > > TaskManager. >>> > > >> >>> > > >> Thanks, >>> > > >> Steve >>> > > >> >>> > > >> On Fri, Jun 10, 2016 at 7:10 AM, Stephan Ewen <se...@apache.org> >>> > wrote: >>> > > >> >>> > > >>> A recent discussion brought up the point of adding a "histogram" >>> > metric >>> > > >>> type to Flink. This open thread is to gather some more of the >>> > > requirements >>> > > >>> for that metric. >>> > > >>> >>> > > >>> The most important question is whether users need Flink to offer >>> > > >>> specific implementations of "Histogram", like for example the " >>> > > >>> com.codahale.metrics.Histogram", or if a " >>> > > >>> org.apache.flink.metrics.Histogram" interface would work as well. >>> > > >>> The histogram could still be reported for example via dropwizard >>> > > >>> reporters. >>> > > >>> >>> > > >>> *Option (1):* If a Flink Histogram works as well, it would be >>> simple >>> > to >>> > > >>> add one. The dropwizard reporter would need to wrap the Flink >>> > > Histogram for >>> > > >>> reporting. >>> > > >>> >>> > > >>> *Option (2)*: If the code needs the specific Dropwizard Histogram >>> > type, >>> > > >>> then one would need a wrapper class that makes a Flink Histogram >>> look >>> > > like >>> > > >>> a dropwizard histogram. >>> > > >>> >>> > > >>> ---------- >>> > > >>> >>> > > >>> As a bit of background for the discussion, here are some thoughts >>> > > behind >>> > > >>> the way that Metrics are currently implemented in Flink. >>> > > >>> >>> > > >>> - The metric types in Flink are independent from libraries like >>> > > >>> "dropwizard" to reduce dependencies and retain freedom to swap >>> > > >>> implementations. >>> > > >>> >>> > > >>> - Metric reporting allows to reuse reporters from dropwizard >>> > > >>> >>> > > >>> - Some Flink metric implementations are also more lightweight >>> than >>> > > for >>> > > >>> example in dropwizard. Counters for example are not thread safe, >>> but >>> > > do not >>> > > >>> impose memory barriers. That is important for metrics deep in the >>> > > streaming >>> > > >>> runtime. >>> > > >>> >>> > > >>> >>> > > >>> >>> > > >> >>> > > > >>> > > > -- >>> > > > -Steve >>> > > > >>> > > > Sent from Gmail Mobile >>> > > > >>> > > >>> > >>> >> >> > -- -Steve Sent from Gmail Mobile
