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. |
+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 <[hidden email]> 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. > > > |
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 <[hidden email]> 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 <[hidden email] > <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 |
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 > |
+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 <[hidden email]> 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 <[hidden email]> > 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 <[hidden email]> 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 <[hidden email]> 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 > > > |
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> > > 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 <[hidden email]> 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 <[hidden email]> > 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 > > > > > > |
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 <[hidden email]> 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 <[hidden email]> > 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 <[hidden email]> 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 <[hidden email]> > > > 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 <[hidden email]> > 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 <[hidden email]> > > 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 > > > > > > > > > > |
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 <[hidden email]> 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 <[hidden email]> > 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 <[hidden email]> >> 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 <[hidden email]> 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 <[hidden email]> >> > > 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 <[hidden email]> >> 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 <[hidden email]> >> > 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 >> > > > >> > > >> > >> > > |
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 <[hidden email] <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 <[hidden email]> > 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 <[hidden email]> >> 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 <[hidden email]> >>> 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 <[hidden email]> >>> 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 <[hidden email] >>> > >>> > > 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 <[hidden email]> >>> 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 <[hidden email]> >>> > 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 |
That is currently not possible. We would have expose the internal
addMetric(String name, Metric metric) method. Regards, Chesnay On 18.06.2016 04:48, Steve Cosenza wrote: > 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 <[hidden email] > <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 > <[hidden email]> 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 > <[hidden email]> 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 > <[hidden email]> > 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 > <[hidden email]> 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 > <[hidden email]> > > > 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 > <[hidden email]> 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 > <[hidden email]> > > 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 > |
Will the added metric be scoped appropriately (e.g. per operator)?
Also, if the added metric is a Counter will it be available when listing counters in AbstractReporter? -Steve On Friday, June 17, 2016, Chesnay Schepler <[hidden email]> wrote: > That is currently not possible. We would have expose the internal > addMetric(String name, Metric metric) method. > > Regards, > Chesnay > > On 18.06.2016 04:48, Steve Cosenza wrote: > > 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 <[hidden email] > <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 <[hidden email] >> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 <[hidden email] >>> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 <[hidden email]> >>>> 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 <[hidden email]> >>>> 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 < >>>> [hidden email] >>>> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> >>>> > > 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 <[hidden email] >>>> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 <[hidden email] >>>> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> >>>> > 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 > > > -- -Steve Sent from Gmail Mobile |
it would be scoped and reported just like any other Counter.
Regards, Chesnay On 18.06.2016 16:04, Steve Cosenza wrote: > Will the added metric be scoped appropriately (e.g. per operator)? > Also, if the added metric is a Counter will it be available when > listing counters in AbstractReporter? > > -Steve > > On Friday, June 17, 2016, Chesnay Schepler <[hidden email] > <mailto:[hidden email]>> wrote: > > That is currently not possible. We would have expose the internal > addMetric(String name, Metric metric) method. > > Regards, > Chesnay > > On 18.06.2016 04:48, Steve Cosenza wrote: >> 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 <[hidden email] >> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 >> <[hidden email] >> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 >> <[hidden email] >> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 >> <[hidden email]> >> 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 >> <[hidden email]> 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 >> <[hidden email] >> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> >> > > 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 >> <[hidden email] >> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> >> 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 >> <[hidden email] >> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> >> > 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 >> > > > > -- > -Steve > > Sent from Gmail Mobile > |
Perfect!
-Steve On Saturday, June 18, 2016, Chesnay Schepler <[hidden email]> wrote: > it would be scoped and reported just like any other Counter. > > Regards, > Chesnay > > On 18.06.2016 16:04, Steve Cosenza wrote: > > Will the added metric be scoped appropriately (e.g. per operator)? > Also, if the added metric is a Counter will it be available when listing > counters in AbstractReporter? > > -Steve > > On Friday, June 17, 2016, Chesnay Schepler < > <javascript:_e(%7B%7D,'cvml','[hidden email]');>[hidden email] > <javascript:_e(%7B%7D,'cvml','[hidden email]');>> wrote: > >> That is currently not possible. We would have expose the internal >> addMetric(String name, Metric metric) method. >> >> Regards, >> Chesnay >> >> On 18.06.2016 04:48, Steve Cosenza wrote: >> >> 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 <[hidden email] >> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 <[hidden email] >>> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 <[hidden email]> >>>> 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 <[hidden email]> >>>>> 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 <[hidden email] >>>>> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 < >>>>> [hidden email] >>>>> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> >>>>> > > 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 <[hidden email] >>>>> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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 <[hidden email] >>>>> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> >>>>> > 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 >> >> >> > > -- > -Steve > > Sent from Gmail Mobile > > > -- -Steve Sent from Gmail Mobile |
Is there a ticket for supporting user defined counters?
Thanks, Steve On Sat, Jun 18, 2016 at 8:18 AM, Steve Cosenza <[hidden email]> wrote: > Perfect! > > -Steve > > > On Saturday, June 18, 2016, Chesnay Schepler <[hidden email]> wrote: > >> it would be scoped and reported just like any other Counter. >> >> Regards, >> Chesnay >> >> On 18.06.2016 16:04, Steve Cosenza wrote: >> >> Will the added metric be scoped appropriately (e.g. per operator)? >> Also, if the added metric is a Counter will it be available when listing >> counters in AbstractReporter? >> >> -Steve >> >> On Friday, June 17, 2016, Chesnay Schepler <[hidden email]> wrote: >> >>> That is currently not possible. We would have expose the internal >>> addMetric(String name, Metric metric) method. >>> >>> Regards, >>> Chesnay >>> >>> On 18.06.2016 04:48, Steve Cosenza wrote: >>> >>> 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 <[hidden email]> 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 <[hidden email]> >>>> 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 <[hidden email]> >>>>> 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 <[hidden email] >>>>>> > >>>>>> 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 <[hidden email]> >>>>>> 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 < >>>>>> [hidden email]> >>>>>> > > 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 <[hidden email]> >>>>>> 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 < >>>>>> [hidden email]> >>>>>> > 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 >>> >>> >>> >> >> -- >> -Steve >> >> Sent from Gmail Mobile >> >> >> > > -- > -Steve > > Sent from Gmail Mobile > |
Hi Steve,
that should be the corresponding JIRA ticket [1]. I think Chesnay already opened a PR for this feature. I think it will be reviewed and probably merged next week. [1] https://issues.apache.org/jira/browse/FLINK-4093 Cheers, Till On Thu, Jun 23, 2016 at 4:33 AM, Steve Cosenza <[hidden email]> wrote: > Is there a ticket for supporting user defined counters? > > Thanks, > Steve > > On Sat, Jun 18, 2016 at 8:18 AM, Steve Cosenza <[hidden email]> > wrote: > >> Perfect! >> >> -Steve >> >> >> On Saturday, June 18, 2016, Chesnay Schepler <[hidden email]> wrote: >> >>> it would be scoped and reported just like any other Counter. >>> >>> Regards, >>> Chesnay >>> >>> On 18.06.2016 16:04, Steve Cosenza wrote: >>> >>> Will the added metric be scoped appropriately (e.g. per operator)? >>> Also, if the added metric is a Counter will it be available when listing >>> counters in AbstractReporter? >>> >>> -Steve >>> >>> On Friday, June 17, 2016, Chesnay Schepler <[hidden email]> wrote: >>> >>>> That is currently not possible. We would have expose the internal >>>> addMetric(String name, Metric metric) method. >>>> >>>> Regards, >>>> Chesnay >>>> >>>> On 18.06.2016 04:48, Steve Cosenza wrote: >>>> >>>> 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 <[hidden email]> 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 <[hidden email]> >>>>> 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 <[hidden email]> >>>>>> 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 < >>>>>>> [hidden email]> >>>>>>> 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 <[hidden email]> >>>>>>> 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 < >>>>>>> [hidden email]> >>>>>>> > > 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 <[hidden email]> >>>>>>> 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 < >>>>>>> [hidden email]> >>>>>>> > 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 >>>> >>>> >>>> >>> >>> -- >>> -Steve >>> >>> Sent from Gmail Mobile >>> >>> >>> >> >> -- >> -Steve >> >> Sent from Gmail Mobile >> > > |
Excellent! Thanks!!!
On Thursday, June 23, 2016, Till Rohrmann <[hidden email]> wrote: > Hi Steve, > > that should be the corresponding JIRA ticket [1]. I think Chesnay already > opened a PR for this feature. I think it will be reviewed and probably > merged next week. > > [1] https://issues.apache.org/jira/browse/FLINK-4093 > > Cheers, > Till > > On Thu, Jun 23, 2016 at 4:33 AM, Steve Cosenza <[hidden email] > <javascript:_e(%7B%7D,'cvml','[hidden email]');>> wrote: > >> Is there a ticket for supporting user defined counters? >> >> Thanks, >> Steve >> >> On Sat, Jun 18, 2016 at 8:18 AM, Steve Cosenza <[hidden email] >> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> wrote: >> >>> Perfect! >>> >>> -Steve >>> >>> >>> On Saturday, June 18, 2016, Chesnay Schepler <[hidden email] >>> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> wrote: >>> >>>> it would be scoped and reported just like any other Counter. >>>> >>>> Regards, >>>> Chesnay >>>> >>>> On 18.06.2016 16:04, Steve Cosenza wrote: >>>> >>>> Will the added metric be scoped appropriately (e.g. per operator)? >>>> Also, if the added metric is a Counter will it be available when >>>> listing counters in AbstractReporter? >>>> >>>> -Steve >>>> >>>> On Friday, June 17, 2016, Chesnay Schepler <[hidden email]> wrote: >>>> >>>>> That is currently not possible. We would have expose the internal >>>>> addMetric(String name, Metric metric) method. >>>>> >>>>> Regards, >>>>> Chesnay >>>>> >>>>> On 18.06.2016 04:48, Steve Cosenza wrote: >>>>> >>>>> 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 <[hidden email]> 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 <[hidden email]> >>>>>> 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 <[hidden email] >>>>>>> > 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 < >>>>>>>> [hidden email]> >>>>>>>> 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 <[hidden email]> >>>>>>>> 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 < >>>>>>>> [hidden email]> >>>>>>>> > > 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 <[hidden email]> >>>>>>>> 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 < >>>>>>>> [hidden email]> >>>>>>>> > 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 >>>>> >>>>> >>>>> >>>> >>>> -- >>>> -Steve >>>> >>>> Sent from Gmail Mobile >>>> >>>> >>>> >>> >>> -- >>> -Steve >>> >>> Sent from Gmail Mobile >>> >> >> > -- -Steve Sent from Gmail Mobile |
Free forum by Nabble | Edit this page |