On Fri, Jan 3, 2014 at 3:33 PM, sebb <seb...@gmail.com> wrote: > On 3 January 2014 13:47, Philippe Mouawad <philippe.moua...@gmail.com> > wrote: > > Hello Sebb, > > As requested although I don't see why it cannot be in bugzilla,if it were > > the history of discussion could be found more easily. > > Because discussion may take a while and Bugzilla is not good for that. > We can add links to the mail thread with any followup, once decisions are > made. > > > Regards > > Philippe > > On Fri, Jan 3, 2014 at 1:57 PM, <bugzi...@apache.org> wrote: > > > >> https://issues.apache.org/bugzilla/show_bug.cgi?id=55932 > >> > >> --- Comment #6 from Sebb <s...@apache.org> --- > >> I have been having a look at the implementation. > >> > >> I don't really see that it needs Commons Math; we aleady have > >> StatCalculator > >> which handles percentiles and more. > >> > > Ok I can change this. > > > >> > >> Likewise, does it really need Commons Pool? > >> It seems wrong to have to have 2 separate pools of SocketOutputStream > >> instances. > >> > > Can you clarify ? > > There is an executor pool (max size : 1 for now) and a > > > > socketOutputStreamPool in GraphiteMetricsManager. > > Sorry, that was wrong. > > There's a socketOutputStreamPool in PickleMetricsManager only > I thought I had spotted another in SocketOutputStreamPoolFactory but > that is just the implementation. > > > > > How many of these would there be? > >> > > > > Currently it is true we could use only one socket and keep it open. > > > > > >> > >> Also, DescriptiveStatistics is not thread-safe (nor is StatCalculator). > >> > > It is not a problem, as DescriptiveStatistics is accessed synchronously. > > Only write accesses are synchronised. > But read needs to be synchronised as well to ensure safe publication. > > >> > >> If we do implement something like this, I think the data processing > needs > >> either to be carefully synchronised, or the raw data should be sent to a > >> separate singleton background thread. > >> > > > > I think it is carefully synchronized in the patch. If not please point me > > to where you see an issue. > > See above. > > If it used a background thread with a suitable queuing mechanism, > there would be no need to synch the data collection, and the > processing would not slow the main thread. > So you mean we publish the SampleResult in a queue and a thread handles computation ? Do you intend to implement a kind of RingBuffer , something like this : https://github.com/LMAX-Exchange/disruptor or much simpler ? And is there another thread to send ? Could you propose a patch change ?
> > What happens currently if the backend socket stalls or runs slowly? > It will only affect the publication . The run method will be executed while the other one is still running , so we could send same statistics twice, good catch. > > Also, decoupling in this way would make it easier to provide > implementations, as they would not need to be thread-safe. > > > > >> > >> Follow-ups to the dev list please. > >> > >> -- > >> You are receiving this mail because: > >> You are the assignee for the bug. > >> > > > > > > > > -- > > Cordialement. > > Philippe Mouawad. > -- Cordialement. Philippe Mouawad.
