kairosdb / kairosdb-client

Java Client for KairosDB
65 stars 67 forks source link

ArrayIndexOutOfBoundsException when re-using MetricBuilder in multiple threads #70

Open martin-g opened 6 years ago

martin-g commented 6 years ago

I reuse the same MetricBuilder to collect a batch/bulk of Metrics before sending them thru the wire but it failed with:

java.lang.ArrayIndexOutOfBoundsException: 887
        at java.util.ArrayList.add(ArrayList.java:463)
        at org.kairosdb.client.builder.MetricBuilder.addMetric(MetricBuilder.java:68)

So it seems MetricBuilder is not prepared to be used in multi-threaded environment.

Are there any recommendations how to do this ? My application does monitoring.monitor("measurement", value, tags) all over the place. I don't want to make single HTTP call for all of them.

jsabin commented 6 years ago

There are really 2 issues here. 1) MetricBuilder is not thread-safe and 2) MetricBuilder is not designed to be reused. Reusing the same MetricBuilder sends the same data points again.

Any reason not to create a MetricBuilder per thread and create a new one each time?

martin-g commented 6 years ago

Yes, I've figured it already! Now I collect the data in a concurrent data structure and once in a while I create MetricBuilder, copy the data to it and then push it. Any new metrics are collected in a new instance of the concurrent data structure in the meantime. I works nice but maybe it would be better if the library provides this functionality for free. As I said: I doubt anyone would want to send the collected metrics one by one. In addition currently KairosClient uses synchronous HttpClient so I use custom thread pool so the send is asynchronous because I do not want the main functionality in my application to wait for the HTTP call. Recently Brian sent an email to Kairos mailing list about monitoring library that works as a logging library (log4j2). There I suggested to take a look at micrometer.io. I use kairos-client in a similar way - I have a wrapper around it that might be used everywhere in the application. A monitoring call may happen at any moment in any thread (actually we have few thousands calls per second), so we need to send the data points in batches and asynchronously. This is also the reason why I've asked recently for a release with the compression support in HttpClient.